Skip to content

Commit

Permalink
Rollup merge of rust-lang#128483 - nnethercote:still-more-cfg-cleanup…
Browse files Browse the repository at this point in the history 
…s, r=petrochenkov

Still more `cfg` cleanups

Found while looking closely at `cfg`/`cfg_attr` processing code.

r? ``````@petrochenkov``````
  • Loading branch information
@jieyouxu
jieyouxu committed Aug 2, 2024
2 parents 2a4bed3 + d1f05fd commit a361a89
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 153 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/cfg_eval.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl CfgEval<'_> {
}

// Now that we have our re-parsed `AttrTokenStream`, recursively configuring
// our attribute target will correctly the tokens as well.
// our attribute target will correctly configure the tokens as well.
flat_map_annotatable(self, annotatable)
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_parse/src/parser/attr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_span::{sym, BytePos, Span};
use thin_vec::ThinVec;
use tracing::debug;

use super::{AttrWrapper, Capturing, FnParseMode, ForceCollect, Parser, PathStyle};
use super::{AttrWrapper, Capturing, FnParseMode, ForceCollect, Parser, ParserRange, PathStyle};
use crate::{errors, fluent_generated as fluent, maybe_whole};

// Public for rustfmt usage
Expand Down Expand Up @@ -313,8 +313,8 @@ impl<'a> Parser<'a> {
// inner attribute, for possible later processing in a `LazyAttrTokenStream`.
if let Capturing::Yes = self.capture_state.capturing {
let end_pos = self.num_bump_calls;
let range = start_pos..end_pos;
self.capture_state.inner_attr_ranges.insert(attr.id, range);
let parser_range = ParserRange(start_pos..end_pos);
self.capture_state.inner_attr_parser_ranges.insert(attr.id, parser_range);
}
attrs.push(attr);
} else {
Expand Down
131 changes: 73 additions & 58 deletions compiler/rustc_parse/src/parser/attr_wrapper.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use rustc_errors::PResult;
use rustc_session::parse::ParseSess;
use rustc_span::{sym, Span, DUMMY_SP};

use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCursor};
use super::{
Capturing, FlatToken, ForceCollect, NodeRange, NodeReplacement, Parser, ParserRange,
TokenCursor,
};

/// A wrapper type to ensure that the parser handles outer attributes correctly.
/// When we parse outer attributes, we need to ensure that we capture tokens
Expand All @@ -28,8 +31,8 @@ use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCurso
#[derive(Debug, Clone)]
pub struct AttrWrapper {
attrs: AttrVec,
// The start of the outer attributes in the token cursor.
// This allows us to create a `ReplaceRange` for the entire attribute
// The start of the outer attributes in the parser's token stream.
// This lets us create a `NodeReplacement` for the entire attribute
// target, including outer attributes.
start_pos: u32,
}
Expand All @@ -53,10 +56,9 @@ impl AttrWrapper {

/// Prepend `self.attrs` to `attrs`.
// FIXME: require passing an NT to prevent misuse of this method
pub(crate) fn prepend_to_nt_inner(self, attrs: &mut AttrVec) {
let mut self_attrs = self.attrs;
mem::swap(attrs, &mut self_attrs);
attrs.extend(self_attrs);
pub(crate) fn prepend_to_nt_inner(mut self, attrs: &mut AttrVec) {
mem::swap(attrs, &mut self.attrs);
attrs.extend(self.attrs);
}

pub fn is_empty(&self) -> bool {
Expand Down Expand Up @@ -89,7 +91,7 @@ struct LazyAttrTokenStreamImpl {
cursor_snapshot: TokenCursor,
num_calls: u32,
break_last_token: bool,
replace_ranges: Box<[ReplaceRange]>,
node_replacements: Box<[NodeReplacement]>,
}

impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
Expand All @@ -104,21 +106,24 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
.chain(iter::repeat_with(|| FlatToken::Token(cursor_snapshot.next())))
.take(self.num_calls as usize);

if self.replace_ranges.is_empty() {
if self.node_replacements.is_empty() {
make_attr_token_stream(tokens, self.break_last_token)
} else {
let mut tokens: Vec<_> = tokens.collect();
let mut replace_ranges = self.replace_ranges.to_vec();
replace_ranges.sort_by_key(|(range, _)| range.start);
let mut node_replacements = self.node_replacements.to_vec();
node_replacements.sort_by_key(|(range, _)| range.0.start);

#[cfg(debug_assertions)]
for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() {
for [(node_range, tokens), (next_node_range, next_tokens)] in
node_replacements.array_windows()
{
assert!(
range.end <= next_range.start || range.end >= next_range.end,
"Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})",
range,
node_range.0.end <= next_node_range.0.start
|| node_range.0.end >= next_node_range.0.end,
"Node ranges should be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})",
node_range,
tokens,
next_range,
next_node_range,
next_tokens,
);
}
Expand All @@ -136,20 +141,23 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
// start position, we ensure that any (outer) replace range which
// encloses another (inner) replace range will fully overwrite the
// inner range's replacement.
for (range, target) in replace_ranges.into_iter().rev() {
assert!(!range.is_empty(), "Cannot replace an empty range: {range:?}");
for (node_range, target) in node_replacements.into_iter().rev() {
assert!(
!node_range.0.is_empty(),
"Cannot replace an empty node range: {:?}",
node_range.0
);

// Replace the tokens in range with zero or one `FlatToken::AttrsTarget`s, plus
// enough `FlatToken::Empty`s to fill up the rest of the range. This keeps the
// total length of `tokens` constant throughout the replacement process, allowing
// us to use all of the `ReplaceRanges` entries without adjusting indices.
// us to do all replacements without adjusting indices.
let target_len = target.is_some() as usize;
tokens.splice(
(range.start as usize)..(range.end as usize),
target
.into_iter()
.map(|target| FlatToken::AttrsTarget(target))
.chain(iter::repeat(FlatToken::Empty).take(range.len() - target_len)),
(node_range.0.start as usize)..(node_range.0.end as usize),
target.into_iter().map(|target| FlatToken::AttrsTarget(target)).chain(
iter::repeat(FlatToken::Empty).take(node_range.0.len() - target_len),
),
);
}
make_attr_token_stream(tokens.into_iter(), self.break_last_token)
Expand Down Expand Up @@ -216,7 +224,7 @@ impl<'a> Parser<'a> {
let cursor_snapshot = self.token_cursor.clone();
let start_pos = self.num_bump_calls;
let has_outer_attrs = !attrs.attrs.is_empty();
let replace_ranges_start = self.capture_state.replace_ranges.len();
let parser_replacements_start = self.capture_state.parser_replacements.len();

// We set and restore `Capturing::Yes` on either side of the call to
// `f`, so we can distinguish the outermost call to
Expand Down Expand Up @@ -271,7 +279,7 @@ impl<'a> Parser<'a> {
return Ok(ret);
}

let replace_ranges_end = self.capture_state.replace_ranges.len();
let parser_replacements_end = self.capture_state.parser_replacements.len();

assert!(
!(self.break_last_token && capture_trailing),
Expand All @@ -288,15 +296,16 @@ impl<'a> Parser<'a> {

let num_calls = end_pos - start_pos;

// Take the captured ranges for any inner attributes that we parsed in
// `Parser::parse_inner_attributes`, and pair them in a `ReplaceRange`
// with `None`, which means the relevant tokens will be removed. (More
// details below.)
let mut inner_attr_replace_ranges = Vec::new();
// Take the captured `ParserRange`s for any inner attributes that we parsed in
// `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() {
if attr.style == ast::AttrStyle::Inner {
if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&attr.id) {
inner_attr_replace_ranges.push((attr_range, None));
if let Some(inner_attr_parser_range) =
self.capture_state.inner_attr_parser_ranges.remove(&attr.id)
{
inner_attr_parser_replacements.push((inner_attr_parser_range, None));
} else {
self.dcx().span_delayed_bug(attr.span, "Missing token range for attribute");
}
Expand All @@ -305,37 +314,41 @@ impl<'a> Parser<'a> {

// This is hot enough for `deep-vector` that checking the conditions for an empty iterator
// is measurably faster than actually executing the iterator.
let replace_ranges: Box<[ReplaceRange]> =
if replace_ranges_start == replace_ranges_end && inner_attr_replace_ranges.is_empty() {
Box::new([])
} else {
// Grab any replace ranges that occur *inside* the current AST node. We will
// perform the actual replacement only when we convert the `LazyAttrTokenStream` to
// an `AttrTokenStream`.
self.capture_state.replace_ranges[replace_ranges_start..replace_ranges_end]
.iter()
.cloned()
.chain(inner_attr_replace_ranges.iter().cloned())
.map(|(range, data)| ((range.start - start_pos)..(range.end - start_pos), data))
.collect()
};
let node_replacements: Box<[_]> = if parser_replacements_start == parser_replacements_end
&& inner_attr_parser_replacements.is_empty()
{
Box::new([])
} else {
// Grab any replace ranges that occur *inside* the current AST node. Convert them
// from `ParserRange` form to `NodeRange` form. We will perform the actual
// replacement only when we convert the `LazyAttrTokenStream` to an
// `AttrTokenStream`.
self.capture_state.parser_replacements
[parser_replacements_start..parser_replacements_end]
.iter()
.cloned()
.chain(inner_attr_parser_replacements.iter().cloned())
.map(|(parser_range, data)| (NodeRange::new(parser_range, start_pos), data))
.collect()
};

// What is the status here when parsing the example code at the top of this method?
//
// When parsing `g`:
// - `start_pos..end_pos` is `12..33` (`fn g { ... }`, excluding the outer attr).
// - `inner_attr_replace_ranges` has one entry (`5..15`, when counting from `fn`), to
// - `inner_attr_parser_replacements` has one entry (`ParserRange(17..27)`), to
// delete the inner attr's tokens.
// - This entry is put into the lazy tokens for `g`, i.e. deleting the inner attr from
// those tokens (if they get evaluated).
// - This entry is converted to `NodeRange(5..15)` (relative to the `fn`) and put into
// the lazy tokens for `g`, i.e. deleting the inner attr from those tokens (if they get
// evaluated).
// - Those lazy tokens are also put into an `AttrsTarget` that is appended to `self`'s
// replace ranges at the bottom of this function, for processing when parsing `m`.
// - `replace_ranges_start..replace_ranges_end` is empty.
// - `parser_replacements_start..parser_replacements_end` is empty.
//
// When parsing `m`:
// - `start_pos..end_pos` is `0..34` (`mod m`, excluding the `#[cfg_eval]` attribute).
// - `inner_attr_replace_ranges` is empty.
// - `replace_range_start..replace_ranges_end` has one entry.
// - `inner_attr_parser_replacements` is empty.
// - `parser_replacements_start..parser_replacements_end` has one entry.
// - One `AttrsTarget` (added below when parsing `g`) to replace all of `g` (`3..33`,
// including its outer attribute), with:
// - `attrs`: includes the outer and the inner attr.
Expand All @@ -346,7 +359,7 @@ impl<'a> Parser<'a> {
num_calls,
cursor_snapshot,
break_last_token: self.break_last_token,
replace_ranges,
node_replacements,
});

// If we support tokens and don't already have them, store the newly captured tokens.
Expand All @@ -367,7 +380,7 @@ impl<'a> Parser<'a> {
// What is the status here when parsing the example code at the top of this method?
//
// When parsing `g`, we add one entry:
// - The `start_pos..end_pos` (`3..33`) entry has a new `AttrsTarget` with:
// - The pushed entry (`ParserRange(3..33)`) has a new `AttrsTarget` with:
// - `attrs`: includes the outer and the inner attr.
// - `tokens`: lazy tokens for `g` (with its inner attr deleted).
//
Expand All @@ -378,12 +391,14 @@ impl<'a> Parser<'a> {
// cfg-expand this AST node.
let start_pos = if has_outer_attrs { attrs.start_pos } else { start_pos };
let target = AttrsTarget { attrs: ret.attrs().iter().cloned().collect(), tokens };
self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target)));
self.capture_state
.parser_replacements
.push((ParserRange(start_pos..end_pos), Some(target)));
} else if matches!(self.capture_state.capturing, Capturing::No) {
// Only clear the ranges once we've finished capturing entirely, i.e. we've finished
// the outermost call to this method.
self.capture_state.replace_ranges.clear();
self.capture_state.inner_attr_ranges.clear();
self.capture_state.parser_replacements.clear();
self.capture_state.inner_attr_parser_ranges.clear();
}
Ok(ret)
}
Expand Down
Loading

0 comments on commit a361a89

Please sign in to comment.