fix the false 'defined here' messages

[henryboisdequin]

Closes #80853.

Take this code:

struct S;

fn repro_ref(thing: S) {
    thing();
}

Previously, the error message would be this:

error[E0618]: expected function, found `S`
 --> src/lib.rs:4:5
  |
3 | fn repro_ref(thing: S) {
  |              ----- `S` defined here
4 |     thing();
  |     ^^^^^--
  |     |
  |     call expression requires function

error: aborting due to previous error

This is incorrect as S is not defined in the function arguments, thing is defined there. With this change, the following is emitted:

error[E0618]: expected function, found `S`
  --> $DIR/80853.rs:4:5
   |
LL | fn repro_ref(thing: S) {
   |              ----- is of type `S`
LL |     thing();
   |     ^^^^^--
   |     |
   |     call expression requires function
   |
   = note: local variable `S` is not a function

error: aborting due to previous error

As you can see, this error message points out that thing is of type S and later in a note, that S is not a function. This change does seem like a downside for some error messages. Take this example:

LL | struct Empty2;
   | -------------- is of type `Empty2`

As you can see, the error message shows that the definition of Empty2 is of type Empty2. Although this isn't wrong, it would be more helpful if it would say something like this (which was there previously):

LL | struct Empty2;
   | -------------- `Empty2` defined here

If there is a better way of doing this, where the Empty2 example would stay the same as without this change, please inform me.

Update: This is now fixed

CC @camelid

[rust-highfive]

r? @varkor

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

[pickfire]

I don't know if adding the note is more helpful since it felt redundant. The error at the top already says so.

expected function, found `Empty2`

And we show

note: unit struct `Empty2` is not a function

If it errors with "expected function" means Empty2 is not a function right? Why do we need to repeat?

[henryboisdequin]

I don't know if adding the note is more helpful since it felt redundant. The error at the top already says so.

expected function, found `Empty2`

And we show

note: unit struct `Empty2` is not a function

If it errors with "expected function" means Empty2 is not a function right? Why do we need to repeat?

Thanks for the review @pickfire. I agree, I'll remove that redundant note. By doing so, the local variable mistake is also fixed.

[henryboisdequin]

@varkor This PR is now ready for review.

[henryboisdequin]

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

[henryboisdequin]

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

[varkor]

If you could address the last two comments and squash the commits, I think this is good to go!

[camelid]

There is another case that I don't think this handles:

const FOO: usize = 0;

fn main() {
    FOO();
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error[E0618]: expected function, found `usize`
 --> src/main.rs:4:5
  |
1 | const FOO: usize = 0;
  | --------------------- `usize` defined here
...
4 |     FOO();
  |     ^^^--
  |     |
  |     call expression requires function

error: aborting due to previous error

For more information about this error, try `rustc --explain E0618`.
error: could not compile `playground`

To learn more, run the command again with --verbose.

[camelid]

Perhaps the fix would be also checking for Res::Def where DefKind::ns is ValueNS?

[henryboisdequin]

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

[henryboisdequin]

Sorry for taking a while, I was on vacation. @varkor, I believed I have addressed all your comments. @camelid I did the check that you have suggested but it does bring us back to the original problem, like this:

LL | struct Empty2;
   | -------------- is of type `Empty2`

It should say Empty2 defined here. Is there a better way to do this? Also, is there a way to get the name of a const to be able to say FOO has type usize instead of -- has type usize. Thanks

[henryboisdequin]

The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)

@pickfire, should I just change it to where it was before

[pickfire]

Looks good to me.

[varkor]

This looks great @henryboisdequin, thanks! Could you squash your commits? Then it's good to go.

[henryboisdequin]

@varkor Commits are squashed ✅

[varkor]

@henryboisdequin: there are some commits that don't appear to be squashed. It's fine to separate different changes into separate commits, but when later commits are reverting changes in previous commits, it just creates churn (which makes tools like git blame less convenient). This change is small enough that it's readable as only one commit.

[henryboisdequin]

@varkor Done

[varkor]

@henryboisdequin: thanks for improving this diagnostic!

@bors r+

[bors]

📌 Commit 7d85592713c495e4b746c06e78bd8f90ab6653cd has been approved by varkor

[henryboisdequin]

@varkor, @camelid Done ✅

[varkor]

@camelid: thanks for pointing that out.

@bors r+

[bors]

📌 Commit d7cb66d has been approved by varkor