-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Detect missing derive
on unresolved attribute even when not imported
#142487
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
r? compiler |
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This needs a perf run firstly I think @bors try parent=855e0fe46e68d94e9f6147531b75ac2d488c548e @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
ugh, that will fail because bors1 doesn't know parents. I meant: @bors2 try parent=855e0fe46e68d94e9f6147531b75ac2d488c548e @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Detect missing `derive` on unresolved attribute even when not imported When encountering unresolved attributes, ensure the proc-macros for every crate in scope are added to the `macro_map` so that typos and missing `derive`s are properly detected. ``` error: cannot find attribute `sede` in this scope --> $DIR/missing-derive-3.rs:20:7 | LL | #[sede(untagged)] | ^^^^ | help: the derive macros `Deserialize` and `Serialize` accept the similarly named `serde` attribute | LL | #[serde(untagged)] | + error: cannot find attribute `serde` in this scope --> $DIR/missing-derive-3.rs:14:7 | LL | #[serde(untagged)] | ^^^^^ | note: `serde` is imported here, but it is a crate, not an attribute --> $DIR/missing-derive-3.rs:4:1 | LL | extern crate serde; | ^^^^^^^^^^^^^^^^^^^ help: `serde` is an attribute that can be used by the derive macros `Deserialize` and `Serialize`, you might be missing a `derive` attribute | LL + #[derive(Deserialize, Serialize)] LL | enum B { | ``` Follow up to #134841.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (07b2f21): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.3%, secondary -0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 464.604s -> 465.331s (0.16%) |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
0d0caca
to
3d10f1b
Compare
This comment was marked as resolved.
This comment was marked as resolved.
r? petrochenkov maybe? |
☔ The latest upstream changes (presumably #145423) made this pull request unmergeable. Please resolve the merge conflicts. |
data.macros | ||
.decode(CrateMetadataRef { cdata: self, cstore }) | ||
.map(|index| DefId { index, krate }) | ||
.collect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can return an iterator using gen { ... }
instead of collecting, see examples in the same file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a neat trick to deal with the lifetimes of the iterator, I didn't know about it. Can't wait for stable generators :)
pub fn all_proc_macro_def_ids(&self) -> Vec<DefId> { | ||
self.iter_crate_data() | ||
.flat_map(|(krate, data)| data.proc_macros_for_crate(krate, self)) | ||
.collect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can return an iterator.
/// find them for suggestions. | ||
pub(crate) fn register_macros_for_all_crates(&mut self) { | ||
if self.all_crate_macros_already_registered { | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: if !things_done { do_things(); things_done = true; }
would look slightly better.
compiler/rustc_resolve/src/lib.rs
Outdated
@@ -1266,6 +1266,10 @@ pub struct Resolver<'ra, 'tcx> { | |||
|
|||
mods_with_parse_errors: FxHashSet<DefId>, | |||
|
|||
/// Whether `Resolver::register_macros_for_all_crates` has been called once already, as we | |||
/// don't need to run it more than once. | |||
all_crate_macros_already_registered: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all_crate_macros_already_registered: bool, | |
all_crate_macros_already_registered: bool = false, |
``` error: cannot find attribute `sede` in this scope --> $DIR/missing-derive-3.rs:20:7 | LL | #[sede(untagged)] | ^^^^ | help: the derive macros `Deserialize` and `Serialize` accept the similarly named `serde` attribute | LL | #[serde(untagged)] | + error: cannot find attribute `serde` in this scope --> $DIR/missing-derive-3.rs:14:7 | LL | #[serde(untagged)] | ^^^^^ | note: `serde` is imported here, but it is a crate, not an attribute --> $DIR/missing-derive-3.rs:4:1 | LL | extern crate serde; | ^^^^^^^^^^^^^^^^^^^ help: `serde` is an attribute that can be used by the derive macros `Deserialize` and `Serialize`, you might be missing a `derive` attribute | LL + #[derive(Deserialize, Serialize)] LL | enum B { | ```
Addressed and squashed. |
When encountering unresolved attributes, ensure the proc-macros for every crate in scope are added to the
macro_map
so that typos and missingderive
s are properly detected.Follow up to #134841. Fix #47608.