Skip to content

Commit

Permalink
Rollup merge of rust-lang#128224 - nnethercote:fewer-replace_ranges, …
Browse files Browse the repository at this point in the history
…r=petrochenkov

Remove unnecessary range replacements

This PR removes an unnecessary range replacement in `collect_tokens_trailing_token`, and does a couple of other small cleanups.

r? ``@petrochenkov``
  • Loading branch information
tgross35 committed Jul 26, 2024
2 parents 5e950ee + 55d37ae commit 048f832
Showing 1 changed file with 21 additions and 25 deletions.
46 changes: 21 additions & 25 deletions compiler/rustc_parse/src/parser/attr_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,15 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
replace_ranges.sort_by_key(|(range, _)| range.start);

#[cfg(debug_assertions)]
{
for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() {
assert!(
range.end <= next_range.start || range.end >= next_range.end,
"Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})",
range,
tokens,
next_range,
next_tokens,
);
}
for [(range, tokens), (next_range, next_tokens)] in replace_ranges.array_windows() {
assert!(
range.end <= next_range.start || range.end >= next_range.end,
"Replace ranges should either be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})",
range,
tokens,
next_range,
next_tokens,
);
}

// Process the replace ranges, starting from the highest start
Expand All @@ -133,9 +131,9 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
// `#[cfg(FALSE)] struct Foo { #[cfg(FALSE)] field: bool }`
//
// By starting processing from the replace range with the greatest
// start position, we ensure that any replace range which encloses
// another replace range will capture the *replaced* tokens for the inner
// range, not the original tokens.
// 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:?}");

Expand Down Expand Up @@ -293,11 +291,13 @@ impl<'a> Parser<'a> {
// with `None`, which means the relevant tokens will be removed. (More
// details below.)
let mut inner_attr_replace_ranges = Vec::new();
for inner_attr in ret.attrs().iter().filter(|a| a.style == ast::AttrStyle::Inner) {
if let Some(attr_range) = self.capture_state.inner_attr_ranges.remove(&inner_attr.id) {
inner_attr_replace_ranges.push((attr_range, None));
} else {
self.dcx().span_delayed_bug(inner_attr.span, "Missing token range for attribute");
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));
} else {
self.dcx().span_delayed_bug(attr.span, "Missing token range for attribute");
}
}
}

Expand Down Expand Up @@ -333,8 +333,7 @@ impl<'a> Parser<'a> {
// 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 two entries.
// - One to delete the inner attribute (`17..27`), obtained when parsing `g` (see above).
// - `replace_range_start..replace_ranges_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 Down Expand Up @@ -365,12 +364,10 @@ 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 two entries:
// When parsing `g`, we add one entry:
// - The `start_pos..end_pos` (`3..33`) entry has a new `AttrsTarget` with:
// - `attrs`: includes the outer and the inner attr.
// - `tokens`: lazy tokens for `g` (with its inner attr deleted).
// - `inner_attr_replace_ranges` contains the one entry to delete the inner attr's
// tokens (`17..27`).
//
// When parsing `m`, we do nothing here.

Expand All @@ -380,7 +377,6 @@ impl<'a> Parser<'a> {
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.replace_ranges.extend(inner_attr_replace_ranges);
} 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.
Expand Down

0 comments on commit 048f832

Please sign in to comment.