Skip to content

Commit

Permalink
Avoid double-handling of attributes in collect_tokens.
Browse files Browse the repository at this point in the history
By keeping track of attributes that have been previously processed.

This has no external effect for now, but is necessary for rust-lang#124141, which
removes nonterminals.
  • Loading branch information
nnethercote committed Aug 21, 2024
1 parent d7455a2 commit 7d65809
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
23 changes: 19 additions & 4 deletions compiler/rustc_parse/src/parser/attr_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,20 @@ impl<'a> Parser<'a> {
res?
};

// Ignore any attributes we've previously processed. This happens when
// an inner call to `collect_tokens` returns an AST node and then an
// outer call ends up with the same AST node without any additional
// wrapping layer.
let ret_attrs: AttrVec = ret
.attrs()
.iter()
.cloned()
.filter(|attr| {
let is_unseen = self.capture_state.seen_attrs.insert(attr.id);
is_unseen
})
.collect();

// When we're not in "definite capture mode", then skip collecting and
// return early if either of the following conditions hold.
// - `None`: Our target doesn't support tokens at all (e.g. `NtIdent`).
Expand All @@ -269,7 +283,7 @@ impl<'a> Parser<'a> {
// tokens.
let definite_capture_mode = self.capture_cfg
&& matches!(self.capture_state.capturing, Capturing::Yes)
&& has_cfg_or_cfg_attr(ret.attrs());
&& has_cfg_or_cfg_attr(&ret_attrs);
if !definite_capture_mode && matches!(ret.tokens_mut(), None | Some(Some(_))) {
return Ok(ret);
}
Expand All @@ -289,7 +303,7 @@ impl<'a> Parser<'a> {
// outer and inner attributes. So this check is more precise than
// the earlier `needs_tokens` check, and we don't need to
// check `R::SUPPORTS_CUSTOM_INNER_ATTRS`.)
|| needs_tokens(ret.attrs())
|| needs_tokens(&ret_attrs)
// - We are in "definite capture mode", which requires that there
// are `#[cfg]` or `#[cfg_attr]` attributes. (During normal
// non-`capture_cfg` parsing, we don't need any special capturing
Expand Down Expand Up @@ -328,7 +342,7 @@ impl<'a> Parser<'a> {
// `Parser::parse_inner_attributes`, and pair them in a `ParserReplacement` with `None`,
// which means the relevant tokens will be removed. (More details below.)
let mut inner_attr_parser_replacements = Vec::new();
for attr in ret.attrs() {
for attr in ret_attrs.iter() {
if attr.style == ast::AttrStyle::Inner {
if let Some(inner_attr_parser_range) =
self.capture_state.inner_attr_parser_ranges.remove(&attr.id)
Expand Down Expand Up @@ -418,7 +432,7 @@ impl<'a> Parser<'a> {
// cfg-expand this AST node.
let start_pos =
if has_outer_attrs { attrs.start_pos.unwrap() } else { collect_pos.start_pos };
let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens };
let target = AttrsTarget { attrs: ret_attrs, tokens };
tokens_used = true;
self.capture_state
.parser_replacements
Expand All @@ -428,6 +442,7 @@ impl<'a> Parser<'a> {
// the outermost call to this method.
self.capture_state.parser_replacements.clear();
self.capture_state.inner_attr_parser_ranges.clear();
self.capture_state.seen_attrs.clear();
}
assert!(tokens_used); // check we didn't create `tokens` unnecessarily
Ok(ret)
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_ast::{
VisibilityKind, DUMMY_NODE_ID,
};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, Diag, FatalError, MultiSpan, PResult};
use rustc_session::parse::ParseSess;
Expand Down Expand Up @@ -183,7 +183,7 @@ pub struct Parser<'a> {
// This type is used a lot, e.g. it's cloned when matching many declarative macro rules with nonterminals. Make sure
// it doesn't unintentionally get bigger.
#[cfg(target_pointer_width = "64")]
rustc_data_structures::static_assert_size!(Parser<'_>, 256);
rustc_data_structures::static_assert_size!(Parser<'_>, 288);

/// Stores span information about a closure.
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -260,6 +260,7 @@ struct CaptureState {
capturing: Capturing,
parser_replacements: Vec<ParserReplacement>,
inner_attr_parser_ranges: FxHashMap<AttrId, ParserRange>,
seen_attrs: FxHashSet<AttrId>,
}

/// Iterator over a `TokenStream` that produces `Token`s. It's a bit odd that
Expand Down Expand Up @@ -457,6 +458,7 @@ impl<'a> Parser<'a> {
capturing: Capturing::No,
parser_replacements: Vec::new(),
inner_attr_parser_ranges: Default::default(),
seen_attrs: Default::default(),
},
current_closure: None,
recovery: Recovery::Allowed,
Expand Down

0 comments on commit 7d65809

Please sign in to comment.