Skip to content

rustc: Search all derives for inert attributes #52230

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

Merged
merged 1 commit into from
Jul 12, 2018

Conversation

alexcrichton
Copy link
Member

This commit fixes an apparent mistake in librustc_resolve where when the
proc_macro feature is enabled (or rust_2018_preview) the resolution of
custom attributes for custom derive was tweaked. Previously when an attribute
failed to resolve it was attempted to locate if there is a custom derive also in
scope which declares the attribute, but only the first custom derive directive
was search.

Instead this commit fixes the loop to search all custom derive invocations
looking for any which register the attribute in question.

Closes #52219

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2018
@alexcrichton
Copy link
Member Author

r? @petrochenkov

@@ -412,7 +428,7 @@ impl<'a> Resolver<'a> {
attrs
});
}
return Err(Determinacy::Undetermined);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think the return was actually correct, but misplaced - we can return once we've found an inert attribute and processed it, we don't need to go through remaining traits and possibly repeat the process, so the return should go inside if inert_attrs.contains(&attr_name) { ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right yes indeed!

Copy link
Member Author

Choose a reason for hiding this comment

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

should be updated now!

@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 Jul 10, 2018
This commit fixes an apparent mistake in librustc_resolve where when the
`proc_macro` feature is enabled (or `rust_2018_preview`) the resolution of
custom attributes for custom derive was tweaked. Previously when an attribute
failed to resolve it was attempted to locate if there is a custom derive also in
scope which declares the attribute, but only the first custom derive directive
was search.

Instead this commit fixes the loop to search all custom derive invocations
looking for any which register the attribute in question.

Closes rust-lang#52219
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 10, 2018

📌 Commit 743a817 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2018
@bors
Copy link
Collaborator

bors commented Jul 12, 2018

⌛ Testing commit 743a817 with merge d334027...

bors added a commit that referenced this pull request Jul 12, 2018
rustc: Search all derives for inert attributes

This commit fixes an apparent mistake in librustc_resolve where when the
`proc_macro` feature is enabled (or `rust_2018_preview`) the resolution of
custom attributes for custom derive was tweaked. Previously when an attribute
failed to resolve it was attempted to locate if there is a custom derive also in
scope which declares the attribute, but only the first custom derive directive
was search.

Instead this commit fixes the loop to search all custom derive invocations
looking for any which register the attribute in question.

Closes #52219
@bors
Copy link
Collaborator

bors commented Jul 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing d334027 to master...

@bors bors merged commit 743a817 into rust-lang:master Jul 12, 2018
@alexcrichton alexcrichton deleted the attr-and-derive branch July 18, 2018 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#![feature(proc_macro)] breaks some custom derive attributes
5 participants