Error message for E0502 can be a little bit more clear/informative ...?

So, in short, from this page, I quote:

With Rust 2018, this error changes for the better:

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
 --> src/main.rs:5:13
  |
4 |     let y = &x;
  |             -- immutable borrow occurs here
5 |     let z = &mut x;
  |             ^^^^^^ mutable borrow occurs here
6 |     
7 |     println!("y: {}", y);
  |                       - borrow later used here

....

How about change the message to:

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
 --> src/main.rs:5:13
  |
4 |     let y = &x;
  |             -- `x` has been immutably borrowed by `y` here
5 |     let z = &mut x;
  |             ^^^^^^ `x` cannot be mutably borrowed again here  
  |                    as it already been immutably borrowed by `y`  
  |                    earlier and will be used later on
6 |     
7 |     println!("y: {}", y);
  |                       - `y` later used here

Just opinion from a user, please ignore if it's too silly.

What if it’s not assigned to a variable? What do you print in the “by” clause then? Also it doesn’t seem like a relevant detail. What matters is the lifetime, not the variable name (if any), and the lifetime is already indicated in the error message.

I don’t think extending the error message to a 3-line-long baroque full sentence helps readability – the converse, in fact.

What’s the rationale for this change? The added words seem to just restate things that are already in another part of the error message or the code snippets it cites.

2 Likes

@lxrec @H2CO3

Yes, the message that I suggested could make the information more redundant and thus not very fitting.

But the main point is to provide more information directly in the code snippets, so it’s easier for users to figure out the problem and thus fix them more quickly.

Some other error message already did it, like this one, so I thought maybe make it a norm?

How about just rewriting the message parts to be clearer, still holding to single lines where possible and still without unnecessary naming of variables?

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
 --> src/main.rs:5:13
  |
4 |     let y = &x;
  |             -- immutable borrow occurs here
5 |     let z = &mut x;
  |             ^^^^^^ impermissible mutable borrow attempted here
6 |     
7 |     println!("y: {}", y);
  |                       - earlier immutable borrow still in use here
1 Like

I think this version is not clearer but actively misleading, because it assumes the second borrow is the “incorrect” one.

In general, any of the three lines of code in the error message might be the incorrect one. If we could know for certain which line was incorrect, we wouldn’t need to present the other two at all.

2 Likes

Good point; the first or third borrow could be of the wrong variable. So just delete impermissible from the second error message. The attempted is still a valid characterization, since it is that attempt which the compiler refuses to compile.

1 Like

I have to agree with the OP’s point that this particular sequence of messages is not clear.

  1. The first two messages do not distinguish between the borrow that does occur and the attempted borrow that the compiler rejects, because in both cases it says “borrow occurs here” (emphasis added)

  2. The third message does not make clear which “borrow (is) later used here”. It relies on the programmer to infer that it is the non-rejected borrow, but since the first and second messages both state that a “borrow occurs here”, it is not clear which borrow that is.

The problem is the second message that incorrectly states that the borrow occurred, rather than stating that the attempted borrow was rejected.

[Edit 1: Added example]

error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
 --> src/main.rs:5:13
  |
4 |     let y = &x;
  |             -- immutable borrow occurs here
5 |     let z = &mut x;
  |             ^^^^^^ attempted mutable borrow rejected here
6 |     
7 |     println!("y: {}", y);
  |                       - earlier immutable borrow still in use here

[Edit 2: Impact on internationalization of Rust]

It may be appropriate to review many of the multipart error messages of rustc, cargo and other tools for clarity to ESL Rustaceans and with an eye to translation to languages that have far fewer programming terms than English. That task might best be accomplished by ESL and other multi-lingual Rustaceans, as they are best positioned to recognize that certain English-language message sequences do not render clearly in other languages in which they are fluent.

[Edit 3: Alternative wording]

Alternatively, the second message could be worded

attempted mutable borrow occurs here, rejected
1 Like

Is it feasible/desirable to have a “learner mode” that outputs more information, like an example of a common E0502 and it’s suggested fix? Experienced users could then get the condensed error output while less experienced users would get an explanation and example fix to help them.

In the case of the “learner mode”, the user would also be told that “since z can modify x and y can view x, modifying z will cause the value of y to change unexpectedly, so the compiler is preventing this kind of logic flaw”

Rustc already outputs

For more information about this error, try `rustc --explain E0502`.

More verbose information, as well as strategies for fixing, should be added there, not in the message.

2 Likes

The "learner mode" idea is more appropriate for situations where the error itself makes perfect sense, but the user doesn't understand how it applies to their specific code, so simply looking up more details on the error message wouldn't help.

My go-to example is that when you get the "Foo doesn't implement Trait" error, you might want to ask "why doesn't this impl block over here cover Foo?" or perhaps "why does calling this function require anything to implement Trait?" or even "why do you think that's a Foo?" all of which would get totally different answers that wouldn't make any sense to put in an extended error description for the original error. Though I have no idea how to make that actually work.

Maybe there are some borrow check errors where this sort of thing could help, but it definitely isn't relevant to this simple example where the explanation is already complete with only three lines of code.

(emphasis changed)

This terminology feels super weird to me, because it sounds like it's saying Rust actually executed this code and encountered a runtime error when it tried to perform the second borrow.

More generally, I don't think it makes sense for the compiler to say that it "rejected" code in an error message. The fact that the code was rejected is already obvious because we're looking at an error message. What we need to say is why it was rejected.

In this case the why is already covered by the main error message at the top, so anything we add to the second inline message is technically redundant (unless we're expecting users to ignore the top line but carefully read all the other lines?). The only thing I can think of that might be an improvement is trying to make the three inline messages flow together a little better.

  |             -- immutable borrow occurs here
  |             ^^^^^^ so cannot borrow mutably here
  |                       - because the immutable borrow is still in use here
3 Likes

The second line in the above three-line error message arises here:

All that is needed to make this error more intelligible, particularly to non-native-English readers, is to change this one line of code from

err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new));

to

err.span_label(span, format!("rejected {} borrow attempted here{}", kind_new, msg_new));
2 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.