bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #128223 - nnethercote:refactor-collect_tokens, r=petrochenkov

Browse source 
Refactor complex conditions in `collect_tokens_trailing_token`

More readability improvements for this complicated function.

r? ````@petrochenkov````
This commit is contained in:
Trevor Gross 2024-07-26 19:03:06 -04:00 • committed by GitHub
commit 553a64f412
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 43 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

11
compiler/rustc_parse/src/parser/attr.rs
View file

@ -457,14 +457,3 @@ impl<'a> Parser<'a> {
Err(self.dcx().create_err(err)) Err(self.dcx().create_err(err))
} }
} }
/// The attributes are complete if all attributes are either a doc comment or a
/// builtin attribute other than `cfg_attr`.
pub fn is_complete(attrs: &[ast::Attribute]) -> bool {
attrs.iter().all(|attr| {
attr.is_doc_comment()
|| attr.ident().is_some_and(|ident| {
ident.name != sym::cfg_attr && rustc_feature::is_builtin_attr_name(ident.name)
})
})
}

77
compiler/rustc_parse/src/parser/attr_wrapper.rs
View file

@ -60,10 +60,6 @@ impl AttrWrapper {
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.attrs.is_empty() self.attrs.is_empty()
} }
pub fn is_complete(&self) -> bool {
crate::parser::attr::is_complete(&self.attrs)
}
} }
/// Returns `true` if `attrs` contains a `cfg` or `cfg_attr` attribute /// Returns `true` if `attrs` contains a `cfg` or `cfg_attr` attribute
@ -199,20 +195,20 @@ impl<'a> Parser<'a> {
force_collect: ForceCollect, force_collect: ForceCollect,
f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>, f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>,
) -> PResult<'a, R> { ) -> PResult<'a, R> {
// Skip collection when nothing could observe the collected tokens, i.e. // We must collect if anything could observe the collected tokens, i.e.
// all of the following conditions hold. // if any of the following conditions hold.
// - We are not force collecting tokens (because force collection // - We are force collecting tokens (because force collection requires
// requires tokens by definition). // tokens by definition).
if matches!(force_collect, ForceCollect::No) let needs_collection = matches!(force_collect, ForceCollect::Yes)
// - None of our outer attributes require tokens. // - Any of our outer attributes require tokens.
&& attrs.is_complete() || needs_tokens(&attrs.attrs)
// - Our target doesn't support custom inner attributes (custom // - Our target supports custom inner attributes (custom
// inner attribute invocation might require token capturing). // inner attribute invocation might require token capturing).
&& !R::SUPPORTS_CUSTOM_INNER_ATTRS || R::SUPPORTS_CUSTOM_INNER_ATTRS
// - We are not in `capture_cfg` mode (which requires tokens if // - We are in `capture_cfg` mode (which requires tokens if
// the parsed node has `#[cfg]` or `#[cfg_attr]` attributes). // the parsed node has `#[cfg]` or `#[cfg_attr]` attributes).
&& !self.capture_cfg || self.capture_cfg;
{ if !needs_collection {
return Ok(f(self, attrs.attrs)?.0); return Ok(f(self, attrs.attrs)?.0);
} }
@ -250,28 +246,28 @@ impl<'a> Parser<'a> {
return Ok(ret); return Ok(ret);
} }
// This is similar to the "skip collection" check at the start of this // This is similar to the `needs_collection` check at the start of this
// function, but now that we've parsed an AST node we have more // function, but now that we've parsed an AST node we have complete
// information available. (If we return early here that means the // information available. (If we return early here that means the
// setup, such as cloning the token cursor, was unnecessary. That's // setup, such as cloning the token cursor, was unnecessary. That's
// hard to avoid.) // hard to avoid.)
// //
// Skip collection when nothing could observe the collected tokens, i.e. // We must collect if anything could observe the collected tokens, i.e.
// all of the following conditions hold. // if any of the following conditions hold.
// - We are not force collecting tokens. // - We are force collecting tokens.
if matches!(force_collect, ForceCollect::No) let needs_collection = matches!(force_collect, ForceCollect::Yes)
// - None of our outer *or* inner attributes require tokens. // - Any of our outer *or* inner attributes require tokens.
// (`attrs` was just outer attributes, but `ret.attrs()` is outer // (`attr.attrs` was just outer attributes, but `ret.attrs()` is
// and inner attributes. That makes this check more precise than // outer and inner attributes. So this check is more precise than
// `attrs.is_complete()` at the start of the function, and we can // the earlier `needs_tokens` check, and we don't need to
// skip the subsequent check on `R::SUPPORTS_CUSTOM_INNER_ATTRS`. // check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.)
&& crate::parser::attr::is_complete(ret.attrs()) || needs_tokens(ret.attrs())
// - We are not in `capture_cfg` mode, or we are but there are no // - We are in `capture_cfg` mode and there are `#[cfg]` or
// `#[cfg]` or `#[cfg_attr]` attributes. (During normal // `#[cfg_attr]` attributes. (During normal non-`capture_cfg`
// non-`capture_cfg` parsing, we don't need any special capturing // parsing, we don't need any special capturing for those
// for those attributes, because they're builtin.) // attributes, because they're builtin.)
&& (!self.capture_cfg || !has_cfg_or_cfg_attr(ret.attrs())) || (self.capture_cfg && has_cfg_or_cfg_attr(ret.attrs()));
{ if !needs_collection {
return Ok(ret); return Ok(ret);
} }
@ -461,6 +457,19 @@ fn make_attr_token_stream(
AttrTokenStream::new(stack_top.inner) AttrTokenStream::new(stack_top.inner)
} }
/// Tokens are needed if:
/// - any non-single-segment attributes (other than doc comments) are present; or
/// - any `cfg_attr` attributes are present;
/// - any single-segment, non-builtin attributes are present.
fn needs_tokens(attrs: &[ast::Attribute]) -> bool {
attrs.iter().any(|attr| match attr.ident() {
None => !attr.is_doc_comment(),
Some(ident) => {
ident.name == sym::cfg_attr || !rustc_feature::is_builtin_attr_name(ident.name)
}
})
}
// Some types are used a lot. Make sure they don't unintentionally get bigger. // Some types are used a lot. Make sure they don't unintentionally get bigger.
#[cfg(target_pointer_width = "64")] #[cfg(target_pointer_width = "64")]
mod size_asserts { mod size_asserts {