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.

Sign up for GitHub

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

Jump to bottom

Look at proc-macro attributes when encountering unknown attribute #109278

Closed
wants to merge 2 commits into from
Closed

Look at proc-macro attributes when encountering unknown attribute #109278

wants to merge 2 commits into from

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 17, 2023 •
edited
Loading 

error: cannot find attribute `sede` in this scope
  --> src/main.rs:18:7
   |
18 |     #[sede(untagged)]
   |       ^^^^
   |
help: the derive macros `Serialize` and `Deserialize` accept the similarly named `serde` attribute
   |
18 |     #[serde(untagged)]
   |       ~~~~~

error: cannot find attribute `serde` in this scope
  --> src/main.rs:12:7
   |
12 |     #[serde(untagged)]
   |       ^^^^^
   |
   = note: `serde` is in scope, but it is a crate, not an attribute
help: `serde` is an attribute that can be used by the derive macros `Serialize` and `Deserialize`, you might be missing a `derive` attribute
   |
10 | #[derive(Serialize, Deserialize)]
   |

Fix #47608.

@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 17, 2023
@rust-log-analyzer

This comment was marked as outdated.

Sign in to view
@estebank estebank force-pushed the serde-attr branch 2 times, most recently from 7c116fc to 51fbd0a Compare March 17, 2023 23:30
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor Author

r? compiler

@rustbot rustbot assigned petrochenkov and unassigned TaKO8Ki Mar 22, 2023
@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 23, 2023 •
edited
Loading

The PR is large and does multiple unrelated things.

Could you first move the emitter change ("show one more line") to a separate PR for someone else to review?
I'm not sure that it is useful, and it's orthogonal to other changes.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2023
@estebank
Look at proc-macro attributes when encountering unknown attribute
43a6cd6 
```
error: cannot find attribute `sede` in this scope
  --> src/main.rs:18:7
   |
18 |     #[sede(untagged)]
   |       ^^^^
   |
help: the derive macros `Serialize` and `Deserialize` accept the similarly named `serde` attribute
   |
18 |     #[serde(untagged)]
   |       ~~~~~

error: cannot find attribute `serde` in this scope
  --> src/main.rs:12:7
   |
12 |     #[serde(untagged)]
   |       ^^^^^
   |
   = note: `serde` is in scope, but it is a crate, not an attribute
help: `serde` is an attribute that can be used by the derive macros `Serialize` and `Deserialize`, you might be missing a `derive` attribute
   |
10 | #[derive(Serialize, Deserialize)]
   |
```

Mitigate rust-lang#47608.
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 30, 2023
@estebank
Copy link
Contributor Author

Moved last commit to #109786.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
estebank
estebank commented Mar 31, 2023
View reviewed changes
compiler/rustc_resolve/src/diagnostics.rs
Comment on lines +1119 to +1121
/// Visit every module reachable from `start_module` and bring any proc and derive macro
/// encountered into `self.macro_map` to be used for suggestions.
fn lookup_macros_from_module(&mut self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that the case in tests/ui/macros/missing-derive-4.rs (use serde;, but no use serde::Serialize;) is handled.

petrochenkov
petrochenkov reviewed Mar 31, 2023
View reviewed changes
compiler/rustc_resolve/src/diagnostics.rs
@@ -1115,6 +1116,83 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

/// Visit every module reachable from `start_module` and bring any proc and derive macro
/// encountered into `self.macro_map` to be used for suggestions.
fn lookup_macros_from_module(&mut self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from lookup_import_candidates_from_module?

Only macros that would be suggested as imports (and their helper attributes) can be suggested here.
So I suggest removing this separate logic and retrieve helper attributes in the lookup_import_candidates_from_module pass if the import candidate happens to be a derive macro and helper attributes satisfy filter_fn.

Same for typo suggestions, in addition for checking any visited item for being typo-suggestable we also need to check its helper attributes for the same thing, as a part of the regular early_lookup_typo_candidate pass.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest landing the core change first - extending the set of regular import and typo suggestions with helper attributes, and leave all the beautifications for later.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 31, 2023
@estebank estebank mentioned this pull request Apr 14, 2023
Closed
@bors
Copy link
Contributor

bors commented May 3, 2023

☔ The latest upstream changes (presumably #110579) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank estebank closed this Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Poor error message when user forgets derive that has attributes
6 participants
@estebank @rustbot @rust-log-analyzer @petrochenkov @bors @TaKO8Ki