-
-
Notifications
You must be signed in to change notification settings - Fork 15k
rustfmt: Format cfg_select!
#154202
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: main
Are you sure you want to change the base?
rustfmt: Format cfg_select!
#154202
Changes from 1 commit
cd3c8cc
2e45542
dd583bf
15f3856
c267330
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backlinks / context for myself (and future travellers): Metadata
HistoryInitial style team discussions
Follow-up: unbraced expressions are permittedPR: #145233 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| use rustc_ast::tokenstream::TokenStream; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: add a backlink to the reference re. //! See <https://doc.rust-lang.org/nightly/reference/conditional-compilation.html#the-cfg_select-macro> for grammar.
|
||
| use rustc_ast::{ | ||
| ast, | ||
| token::{self, Token}, | ||
| }; | ||
| use rustc_parse::exp; | ||
| use rustc_span::Span; | ||
| use tracing::debug; | ||
|
|
||
| use crate::parse::session::ParseSess; | ||
| use crate::spanned::Spanned; | ||
|
|
||
| pub(crate) enum CfgSelectFormatPredicate { | ||
| Cfg(ast::MetaItemInner), | ||
| Wildcard(Span), | ||
| } | ||
|
Comment on lines
+13
to
+16
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: maybe leave a quick TL;DR example for this: /// LHS predicate of a `cfg_select!` arm.
pub(crate) enum CfgSelectFormatPredicate {
/// Example: the `unix` in `unix => {}`. Notably, outer or inner attributes are not permitted.
Cfg(ast::MetaItemInner),
/// `_` in `_ => {}`.
Wildcard(Span),
}Re. outer/inner attributes, counter-example: cfg_select! {
#[cfg(true)]
_ => {
fn main() {}
}
} |
||
|
|
||
| impl Spanned for CfgSelectFormatPredicate { | ||
| fn span(&self) -> rustc_span::Span { | ||
| match self { | ||
| Self::Cfg(meta_item_inner) => meta_item_inner.span(), | ||
| Self::Wildcard(span) => *span, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub(crate) struct CfgSelectArm { | ||
| pub(crate) predicate: CfgSelectFormatPredicate, | ||
| pub(crate) arrow: Token, | ||
| pub(crate) expr: Box<ast::Expr>, | ||
| pub(crate) trailing_comma: Option<Span>, | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: maybe some comments for "visual" purposes. /// Each `$predicate => $production` arm in `cfg_select!`.
pub(crate) struct CfgSelectArm {
/// The `$predicate` part.
pub(crate) predicate: CfgSelectFormatPredicate,
/// Span of `=>`.
pub(crate) arrow: Token,
/// The RHS `$production` expression.
pub(crate) expr: Box<ast::Expr>,
/// `cfg_select!` arms `$production`s can be optionally `,` terminated, like `match` arms. The `,` is not needed when `$production` is itself braced `{}`.
pub(crate) trailing_comma: Option<Span>,
} |
||
|
|
||
| impl PartialEq for &CfgSelectArm { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| // consider the arms equal if they have the same span | ||
| self.span() == other.span() | ||
| } | ||
| } | ||
|
|
||
| impl Spanned for CfgSelectArm { | ||
| fn span(&self) -> Span { | ||
| self.predicate | ||
| .span() | ||
| .with_hi(if let Some(comma) = self.trailing_comma { | ||
| comma.hi() | ||
| } else { | ||
| self.expr.span.hi() | ||
| }) | ||
| } | ||
| } | ||
|
Comment on lines
+41
to
+51
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: could you double-check if the following cross-crate macro expansion case // another_crate.rs
macro_rules! garbage {
() => {
fn main() {}
}
}
// main.rs
cfg_select! {
_ => {
garbage!();
}
}produces a span that can be used for formatting? |
||
|
|
||
| impl std::fmt::Debug for CfgSelectArm { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| match &self.predicate { | ||
| CfgSelectFormatPredicate::Cfg(cfg_entry) => cfg_entry.fmt(f)?, | ||
| CfgSelectFormatPredicate::Wildcard(t) => t.fmt(f)?, | ||
| }; | ||
| write!(f, "=> {:?}", self.expr) | ||
| } | ||
| } | ||
|
|
||
| // FIXME(ytmimi) would be nice if rustfmt didn't need to implement parsing logic on its own | ||
| // and could instead just call rustc_attr_parsing::parse_cfg_select, but this is fine for now. | ||
| pub(crate) fn parse_cfg_select(psess: &ParseSess, ts: TokenStream) -> Option<Vec<CfgSelectArm>> { | ||
| let mut cfg_select_predicates = vec![]; | ||
| let mut parser = super::build_stream_parser(psess.inner(), ts); | ||
|
|
||
| while parser.token != token::Eof { | ||
| let predicate = if parser.eat_keyword(exp!(Underscore)) { | ||
| CfgSelectFormatPredicate::Wildcard(parser.prev_token.span) | ||
| } else { | ||
| let Ok(meta_item) = parser.parse_meta_item_inner().map_err(|e| e.cancel()) else { | ||
| debug!("Failed to parse cfg entry in cfg_select! predicate"); | ||
| return None; | ||
| }; | ||
| CfgSelectFormatPredicate::Cfg(meta_item) | ||
| }; | ||
|
|
||
| if let Err(_) = parser.expect(exp!(FatArrow)) { | ||
| debug!("Expected to find a `=>` after cfg_selec! predicate."); | ||
| return None; | ||
| }; | ||
|
|
||
| let arrow = parser.prev_token; | ||
|
|
||
| let Ok(expr) = parser.parse_expr().map_err(|e| e.cancel()) else { | ||
| debug!("Couldn't parse cfg_select! arm body after `=>`."); | ||
| return None; | ||
| }; | ||
|
|
||
| let trailing_comma = if parser.eat(exp!(Comma)) { | ||
| Some(parser.prev_token.span) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let arm = CfgSelectArm { | ||
| predicate, | ||
| arrow, | ||
| expr, | ||
| trailing_comma, | ||
| }; | ||
|
|
||
| cfg_select_predicates.push(arm); | ||
| } | ||
| Some(cfg_select_predicates) | ||
| } | ||
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.
Question: just to check my understanding,
I assume this is because
rustfmtcan't / don't want to (transitively) depend on things beyond parsing/AST? I.e.rustc_hir?