Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostics: improve error message for incorrect inner attribute #89566

Closed
ogham opened this issue Oct 5, 2021 · 4 comments · Fixed by #118533
Closed

Diagnostics: improve error message for incorrect inner attribute #89566

ogham opened this issue Oct 5, 2021 · 4 comments · Fixed by #118533
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ogham
Copy link
Contributor

ogham commented Oct 5, 2021

I was assisting someone new to Rust and they were confused by an error message. This is the complete file of code they had written:

#![derive(Debug)]
struct Test {
    s: String,
}

fn main() {
}

Using rustc 1.55.0 (c8dfcfe04 2021-09-06) and rustc 1.57.0-nightly (f03eb6b 2021-10-02), the compiler outputs the following error (playground link):

error: cannot determine resolution for the attribute macro `derive`
 --> src/main.rs:1:4
  |
1 | #![derive(Debug)]
  |    ^^^^^^
  |
  = note: import resolution is stuck, try simplifying macro imports

The fix is to use #[derive], an outer attribute, rather than #![derive], an inner attribute, but the compiler's error does not make that clear. And its suggestion to simplify macro imports doesn't help, as from the user's perspective, nothing is being imported in this source file!

I think a more specific error message that checks whether the inner attribute's name would fit as an outer attribute would be helpful here. A suggestion to remove the ! would be even better. Annoyingly, having #! at the start of the file looked correct as it resembles a shebang.

Note that there is a good error for using #![] instead of #[] in the general case. It even mentions the difference between inner and outer attributes in its message. It was just bad luck that this particular struct happened to be at the very start of the source file, so that error was not displayed.

I also spotted that in issue #67107, which has a very similar piece of example code, the compiler used to provide a suggestion to "try an outer attribute". But, running that example on the latest stable and nightly, it looks like that suggestion is not being produced anymore.

@ogham ogham added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 5, 2021
@estebank estebank added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Oct 7, 2021
@tom7980
Copy link
Contributor

tom7980 commented Oct 7, 2021

I'm fairly sure I know what is causing this however I believe that it will require a new Error message as the issue is that the parser is parsing #![derive] as an inner attribute of the crate itself and then injecting the std library below it between it and the struct.

@estebank I'm not 100% on the process of designing new error messages for rust but I wanted to see if this makes sense:

A) the !#[derive] is at the bottom of the "crate attributes" so we can update the parser so it ignores any !#[derive] attributes while parsing the initial set of attributes for the crate & carry on from there no problem

B) the !#[derive] is in the middle of some other attributes (not sure how or why but it might) and we need to error, I'm thinking - Error: !#[derive] cannot be used as a crate attribute (point to the derive via it's span)

I think the only issue I have with this is that I feel I may need to spend a while trying to understand the parser to fix this one - it already seems like it will be difficult to selectively parse attributes when I don't want to or even to tell what kind of attributes I have actually parsed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 4, 2021
…nkov

Updated error message for accidental uses of derive attribute as a crate attribute

This partially fixes the original issue rust-lang#89566 by adding derive to the list of invalid crate attributes and then providing an updated error message however I'm not sure how to prevent the resolution error message from emitting without causing the compiler to just abort when it finds an invalid crate attribute (which I'd prefer not to do so we can find and emit other errors).

`@petrochenkov` I have been told you may have some insight on why it's emitting the resolution error though honestly I'm not sure if we need to worry about fixing it as long as we can provide the invalid crate attribute error also (which happens first anyway)
@estebank
Copy link
Contributor

estebank commented Oct 18, 2023

Current output:

error: cannot determine resolution for the attribute macro `derive`
 --> src/main.rs:1:4
  |
1 | #![derive(Debug)]
  |    ^^^^^^
  |
  = note: import resolution is stuck, try simplifying macro imports

error: `derive` attribute cannot be used at crate level
 --> src/main.rs:1:1
  |
1 | #![derive(Debug)]
  | ^^^^^^^^^^^^^^^^^
  |
help: perhaps you meant to use an outer attribute
  |
1 | #[derive(Debug)]
  |

All we need to do is try to silence the first error, if possible, if the second one is emitted.

If there's an item before it the error is better:

error: an inner attribute is not permitted in this context
 --> src/main.rs:2:1
  |
2 |   #![derive(Debug)]
  |   ^^^^^^^^^^^^^^^^^
3 | / struct Test {
4 | |     s: String,
5 | | }
  | |_- the inner attribute doesn't annotate this struct
  |
  = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files
help: to annotate the struct, change the attribute from inner to outer style
  |
2 - #![derive(Debug)]
2 + #[derive(Debug)]
  |

@tom7980
Copy link
Contributor

tom7980 commented Oct 18, 2023

Hi Estebank, long time - I think I remember hitting a bit of a wall with suppressing the first error as it was pretty deeply embedded in the resolver stage of the compiler & if I wanted to suppress it I had to abort any step that would allow me to output a useful error about the attribute symbol not being usable in that location.

I think the issue with having to suppress the first error is that the context of resolution of inner attributes has not been fully defined yet #54726 goes into this more but until that is resolved (pardon the pun) I'm not sure we can suppress the first error. Although potentially we can just look for the first error being emitted and just not emit it? Not sure if that is possible?

@estebank
Copy link
Contributor

It'll be difficult to silence the first error, because that one happens during macro expansion, while the later occurs in a much later stage. For now, I feel that it is ok to have both errors, but let's leave this ticket open to maybe address in the future.

@estebank estebank removed D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Oct 18, 2023
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this issue Oct 27, 2023
Tweak suggestion span for outer attr and point at item following invalid inner attr

After:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL |
LL | fn main() {}
   | ------------ the inner attribute doesn't annotate this function
   |
help: perhaps you meant to use an outer attribute
   |
LL - #![unix_sigpipe = "inherit"]
LL + #[unix_sigpipe = "inherit"]
   |
```

Before:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: perhaps you meant to use an outer attribute
   |
LL | #[unix_sigpipe = "inherit"]
   | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

CC rust-lang#89566.
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this issue Oct 27, 2023
Tweak suggestion span for outer attr and point at item following invalid inner attr

After:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL |
LL | fn main() {}
   | ------------ the inner attribute doesn't annotate this function
   |
help: perhaps you meant to use an outer attribute
   |
LL - #![unix_sigpipe = "inherit"]
LL + #[unix_sigpipe = "inherit"]
   |
```

Before:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: perhaps you meant to use an outer attribute
   |
LL | #[unix_sigpipe = "inherit"]
   | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

CC rust-lang#89566.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 27, 2023
Tweak suggestion span for outer attr and point at item following invalid inner attr

After:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL |
LL | fn main() {}
   | ------------ the inner attribute doesn't annotate this function
   |
help: perhaps you meant to use an outer attribute
   |
LL - #![unix_sigpipe = "inherit"]
LL + #[unix_sigpipe = "inherit"]
   |
```

Before:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: perhaps you meant to use an outer attribute
   |
LL | #[unix_sigpipe = "inherit"]
   | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

CC rust-lang#89566.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 27, 2023
Rollup merge of rust-lang#116868 - estebank:suggestion, r=petrochenkov

Tweak suggestion span for outer attr and point at item following invalid inner attr

After:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL |
LL | fn main() {}
   | ------------ the inner attribute doesn't annotate this function
   |
help: perhaps you meant to use an outer attribute
   |
LL - #![unix_sigpipe = "inherit"]
LL + #[unix_sigpipe = "inherit"]
   |
```

Before:

```
error: `unix_sigpipe` attribute cannot be used at crate level
  --> $DIR/unix_sigpipe-crate.rs:2:1
   |
LL | #![unix_sigpipe = "inherit"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
help: perhaps you meant to use an outer attribute
   |
LL | #[unix_sigpipe = "inherit"]
   | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

CC rust-lang#89566.
chenyukang added a commit to chenyukang/rust that referenced this issue Dec 2, 2023
chenyukang added a commit to chenyukang/rust that referenced this issue Jan 4, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 28, 2024
…trochenkov

Suppress unhelpful diagnostics for unresolved top level attributes

Fixes rust-lang#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.

Also fixes rust-lang#89566, fixes rust-lang#67107.

r? `@petrochenkov`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 28, 2024
…trochenkov

Suppress unhelpful diagnostics for unresolved top level attributes

Fixes rust-lang#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.

Also fixes rust-lang#89566, fixes rust-lang#67107.

r? ``@petrochenkov``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 28, 2024
…trochenkov

Suppress unhelpful diagnostics for unresolved top level attributes

Fixes rust-lang#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.

Also fixes rust-lang#89566, fixes rust-lang#67107.

r? ```@petrochenkov```
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 30, 2024
…ochenkov

Suppress unhelpful diagnostics for unresolved top level attributes

Fixes rust-lang#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.

Also fixes rust-lang#89566, fixes rust-lang#67107.

r? `@petrochenkov`
@bors bors closed this as completed in ee2e9e1 Jan 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 30, 2024
Rollup merge of rust-lang#118533 - chenyukang:yukang-fix-118455, r=petrochenkov

Suppress unhelpful diagnostics for unresolved top level attributes

Fixes rust-lang#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.

Also fixes rust-lang#89566, fixes rust-lang#67107.

r? `@petrochenkov`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants