E0716: clarify that equivalent code example is erroneous

[tlyu]

In E0716, there is a code block that is equivalent to the erroneous
code example. Especially when viewed with rustc --explain, it's
not obvious that it is also erroneous, and some users have been
confused when they try to change their code to match the erroneous
equivalent.

@rustbot label +A-diagnostics +D-newcomer-roadblock +T-compiler

[rust-highfive]

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

[JohnTitor]

This (equivalent to.. vs. equivalent erroneous code) sounds slightly redundant, maybe we could improve the above wording instead.

[tlyu]

Thanks! I think there needs to be a very clear indication that the example is erroneous, preferably on a line by itself. I know it seems redundant, but somehow the erroneous nature of the example to be clearly communicated when the explanation is displayed in plain text in a terminal. I think it's very easy for someone quickly scanning the --explain output in a terminal to assume that the second code block is an example of how to fix their code. (I think I've heard one or two examples in Discord of someone copy and pasting such an example, maybe even this one, and being upset when it didn't work.)

Right now the rustc --explain E0716 begins with

A temporary value is being dropped while a borrow is still in active use.

Erroneous code example:

```
fn foo() -> i32 { 22 }
fn bar(x: &i32) -> &i32 { x }
let p = bar(&foo());
         // ------ creates a temporary
let q = *p;
```

Here, the expression `&foo()` is borrowing the expression `foo()`. As `foo()` is
a call to a function, and not the name of a variable, this creates a
**temporary** -- that temporary stores the return value from `foo()` so that it
can be borrowed. You could imagine that `let p = bar(&foo());` is equivalent to
this:

```
let p = {
  let tmp = foo(); // the temporary
  bar(&tmp)
}; // <-- tmp is freed as we exit this block
let q = p;
```

Note there is nothing obviously highlighting the second code block as erroneous, not even the minimal highlighting that compile_fail examples get in the error index web page. Even the compile_fail annotation that's in the .md file gets stripped from the code fence. I guess we could repeat the wording before the first code block and say

Erroneous code example:

on a line by itself? Maybe also change the comment to something like this?

}; // <-- ERROR: tmp is freed as we exit this block

[JohnTitor]

Hm, I think "You could imagine that let p = bar(&foo()); is equivalent to this:" should mean it's also erroneous. If it's unclear, it's better to reword that instead of just saying it's equivalent again otherwise it'll be repeating the same wording with the same meaning. I'd reword something like "You could imagine that let p = bar(&foo()); is equivalent to this which also fails".
And note that "tmp is freed as we exit this block" is not the error itself, it explains why the error happened.

[tlyu]

How do you feel about putting another

Erroneous code example:

line in front of the second erroneous code block? I think making it any more verbose, or trying to have the explanatory paragraph serve the purpose of highlighting the erroneous nature, will make the code block more likely to be misinterpreted as a fix.

Also, the existing error output (from the playground) is very close to what the existing comment in the example says.

error[E0597]: `tmp` does not live long enough
 --> src/main.rs:7:7
  |
5 | let p = {
  |     - borrow later stored here
6 |   let tmp = foo(); // the temporary
7 |   bar(&tmp)
  |       ^^^^ borrowed value does not live long enough
8 | }; // <-- tmp is freed as we exit this block
  | - `tmp` dropped here while still borrowed

RFC 1567 says to put a comment on the erroneous line, which I guess is line 7 here? Either way, it seems to recommend explicitly putting // error: in a comment.

[JohnTitor]

How do you feel about putting another [..] line in front of the second erroneous code block? I think making it any more verbose, or trying to have the explanatory paragraph serve the purpose of highlighting the erroneous nature, will make the code block more likely to be misinterpreted as a fix.

I'm not sure what you mean, how my suggestion causes misunderstanding? It's just saying the following also fails. And note that even though you add any wording before the snippet, you should reword "You could imagine.." part to avoid being verbose.

Also, the existing error output (from the playground) is very close to what the existing comment in the example says.

You can see the error happens on line 7, which means using a temporary value was used and line 8 just dropped tmp so "ERROR: tmp is freed as we exit this block" is totally incorrect. I'm not saying that adding ERROR: is bad but line 8 isn't the error itself.

[tlyu]

I'm not sure what you mean, how my suggestion causes misunderstanding? It's just saying the following also fails. And note that even though you add any wording before the snippet, you should reword "You could imagine.." part to avoid being verbose.

I'm saying that I think we still need some brief text on a line by itself in front of the erroneous code example that clearly says that it's erroneous, otherwise users will misinterpret it as an example of how to fix the code. I think this is regardless of what wording changes we make in the "You could imagine" paragraph. I think that paragraph is long enough that users might skip over it without reading it, especially if they see something that looks like an example of a fix.

Anyway, I pushed an update; please let me know what you think?

[JohnTitor]

I'm not sure if the current is the best but it seems fine, could you squash commits into one?

[tlyu]

I'm not sure if the current is the best but it seems fine, could you squash commits into one?

Done. Thanks!

@rustbot label -S-waiting-on-author +S-waiting-on-review

[JohnTitor]

Thanks!
r? @JohnTitor @bors r+ rollup

[bors]

📌 Commit 92197a5 has been approved by JohnTitor