-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Refactor find_*_stability
functions
#109539
Conversation
r? @b-naber (rustbot has picked a reviewer for you, use r? to override) |
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.
Looks a lot better. r=me with or without the nits.
compiler/rustc_attr/src/builtin.rs
Outdated
} | ||
} | ||
} | ||
/// Collects stability info from all stability attributes in `attrs`. |
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 we update these comments or remove the first sentence? I feel that in their current form these aren't really helpful.
body_stab | ||
} | ||
|
||
fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, StabilityLevel)> { |
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.
Maybe add a brief doc comment for these?
compiler/rustc_attr/src/builtin.rs
Outdated
fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, StabilityLevel)> { | ||
let meta = attr.meta()?; | ||
let MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else { return None }; | ||
let insert = |meta: &MetaItem, item: &mut Option<Symbol>| { |
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.
Maybe choose a more expressive name for this closure?
@bors r+ rollup |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#109355 (Fix bad suggestion for clone/is_some in field init shorthand) - rust-lang#109484 (Bugfix: avoid panic on invalid json output from libtest) - rust-lang#109539 (Refactor `find_*_stability` functions) - rust-lang#109542 (rustdoc: clean up `storage.js`) - rust-lang#109545 (Deeply check well-formedness of return-position `impl Trait` in trait) - rust-lang#109568 (miri: fix raw pointer dyn receivers) - rust-lang#109570 (Add GUI test for "Auto-hide item methods' documentation" setting) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The idea is to split the monolithic function into the 3 cases: stability, const stability, and body stability.