Simplify attribute_groups
#140539
Simplify attribute_groups
#140539
Conversation
rustbot
added
A-attributes
|
Some changes occurred in compiler/rustc_attr_parsing |
nnethercote
force-pushed
the
simplify-attribute_groups
branch
from
923fc5e to
e1ab116
Compare
There was a problem hiding this comment.
This is not a full review, was just skimming through, but I don't think simplifying the parsers to take one symbol is that nice since attributes can be namespaced. We don't do this a whole lot right now, but I wish we did more. Diagnostics attributes use this for example.
| $( | ||
| { | ||
| thread_local! { | ||
| static STATE_OBJECT: RefCell<$names> = RefCell::new(<$names>::default()); | ||
| }; | ||
|
|
||
| for (k, v) in <$names>::ATTRIBUTES { | ||
| accepts.entry(*k).or_default().push(Box::new(|cx, args| { | ||
| let old = accepts.insert(*k, Box::new(|cx, args| { |
There was a problem hiding this comment.
fair, used to be a vec I think but this is better
| @@ -33,7 +33,7 @@ pub(crate) mod transparency; | |||
| pub(crate) mod util; | |||
|
|
|||
| type AcceptFn<T> = fn(&mut T, &AcceptContext<'_>, &ArgParser<'_>); | |||
| type AcceptMapping<T> = &'static [(&'static [rustc_span::Symbol], AcceptFn<T>)]; | |||
| type AcceptMapping<T> = &'static [(Symbol, AcceptFn<T>)]; | |||
There was a problem hiding this comment.
We can't do this. There are attributes that require more than one symbol in its path, and doing this would mean we can't parse those anymore, which would be a shame. None of the first few parsers I built use this feature, but I think it is an important one
There was a problem hiding this comment.
Like namespaced attributes?
EDIT: as examples: #[diagnostic::{do_not_recommend,on_unimplemented}]
|
we briefly discussed this during rustweek I believe, but coming back to this: I'm happy to accept all the comment changes and some more, but none of the symbol slice -> single symbol changes. |
|
@rustbot author (re. direction) |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
There only needs to be one `Fn` per symbol, not multiple.
Some are too long (> 100 chars), some are too short, some are missing full stops, some are missing upper-case letters at the start of sentences.
In several files they are entirely unnecessary, with the relevant names already imported. And in a few I have added the necessary `use` item.
nnethercote
force-pushed
the
simplify-attribute_groups
branch
from
e1ab116 to
354b1cb
Compare
|
@jdonszelmann: I have removed the commit that changed the symbol slice to a single symbol. I have added a new commit that removes some unnecessary/undesirable @rustbot ready |
rustbot
added
S-waiting-on-review
|
perfect! @bors r+ |
bors
added
S-waiting-on-bors
|
@bors rollup |
…ups, r=jdonszelmann Simplify `attribute_groups` It's more complicated than it needs to be. r? `@jdonszelmann`
Rollup of 9 pull requests Successful merges: - #134696 (Implement `normalize_lexically`) - #138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of #94678)) - #140539 (Simplify `attribute_groups`) - #140863 ([rustdoc] Unify type aliases rendering with other ADT) - #140936 (Clarify WTF-8 safety docs) - #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace) - #141472 (Attempt to improve the `std::fs::create_dir_all` docs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild) - #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 8 pull requests Successful merges: - #134696 (Implement `normalize_lexically`) - #140539 (Simplify `attribute_groups`) - #140863 ([rustdoc] Unify type aliases rendering with other ADT) - #140936 (Clarify WTF-8 safety docs) - #140952 (Specify that split_ascii_whitespace uses the same definition as is_ascii_whitespace) - #141472 (Attempt to improve the `std::fs::create_dir_all` docs related to atomicity) - #141502 (ci: move PR job x86_64-gnu-tools to codebuild) - #141559 (const-check: stop recommending the use of rustc_allow_const_fn_unstable) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #140539 - nnethercote:simplify-attribute_groups, r=jdonszelmann Simplify `attribute_groups` It's more complicated than it needs to be. r? ``@jdonszelmann``
It's more complicated than it needs to be.
r? @jdonszelmann