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
Use ast_map in ty::get_attrs to enable querying attributes for any type of node. #23028
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Aatch (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
I wonder if this should just return Oh, and, that match is already done in |
So I guess we should just replace the contents of Change |
@eddyb have your points been addressed? |
@@ -459,33 +459,30 @@ impl<'ast> Map<'ast> { | |||
|
|||
/// Given a node ID and a closure, apply the closure to the array |
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.
This shouldn't mention the closure anymore.
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, was there nothing using this function? 0 fallout?
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.
Nope, nothing at all, but I'd venture that it's still a useful function for plugins and such.
We don't need to take a closure, instead just return the list of attributes.
This is more flexible and less error-prone. `get_attrs` can now be used on many more types of items.
a262807
to
caf6f17
Compare
Well now the pull request isn't really aptly named anymore, but the commits are cleaner. |
@bors rollup r+ |
This is more flexible and less error-prone. `get_attrs` and `get_attrs_opt` can be used on many more items than the old `get_attrs` could. This is all courtesy of @huonw, and directly taken from here: https://github.com/rust-lang/rust/pull/22348/files#diff-0f85fcb07fb739876892e633fa0e2be6R5575 Also thanks to @Manishearth for pointing it out to me.
This is more flexible and less error-prone.
get_attrs
andget_attrs_opt
can be used on many more items than the oldget_attrs
could.
This is all courtesy of @huonw, and directly taken from here:
https://github.com/rust-lang/rust/pull/22348/files#diff-0f85fcb07fb739876892e633fa0e2be6R5575
Also thanks to @Manishearth for pointing it out to me.