Skip to content

Commit d7455a2

Browse files
committed
Avoid nested replacement ranges.
In a case like this: ``` mod a { mod b { #[cfg_attr(unix, inline)] fn f() { #[cfg_attr(linux, inline)] fn g1() {} #[cfg_attr(linux, inline)] fn g2() {} } } } ``` We currently end up with the following replacement ranges. - The lazy tokens for `f` has replacement ranges for `g1` and `g2`. - The lazy tokens for `a` has replacement ranges for `f`, `g1`, and `g2`. I.e. the replacement ranges for `g1` and `g2` are duplicated. In general, replacement ranges for inner AST nodes are duplicated up the chain for each nested `collect_tokens` call. And the code that processes the replacements is careful about the ordering in which the replacements are applied, to ensure that inner replacements are applied before outer replacements. But all of this is unnecessary. If you apply an inner replacement and then an outer replacement, the outer replacement completely overwrites the inner replacement. This commit avoids the duplication by removing replacements from `self.capture_state.parser_replacements` when they are used. (The effect on the example above is that the lazy tokesn for `a` no longer include replacement ranges for `g1` and `g2`.) This eliminates the possibility of nested replacements on individual AST nodes, which avoids the need for careful ordering of replacements.
1 parent 889ed02 commit d7455a2

File tree

1 file changed

+7
-21
lines changed

1 file changed

+7
-21
lines changed

compiler/rustc_parse/src/parser/attr_wrapper.rs

+7-21
Original file line numberDiff line numberDiff line change
@@ -134,30 +134,17 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
134134
node_replacements.array_windows()
135135
{
136136
assert!(
137-
node_range.0.end <= next_node_range.0.start
138-
|| node_range.0.end >= next_node_range.0.end,
139-
"Node ranges should be disjoint or nested: ({:?}, {:?}) ({:?}, {:?})",
137+
node_range.0.end <= next_node_range.0.start,
138+
"Node ranges should be disjoint: ({:?}, {:?}) ({:?}, {:?})",
140139
node_range,
141140
tokens,
142141
next_node_range,
143142
next_tokens,
144143
);
145144
}
146145

147-
// Process the replace ranges, starting from the highest start
148-
// position and working our way back. If have tokens like:
149-
//
150-
// `#[cfg(FALSE)] struct Foo { #[cfg(FALSE)] field: bool }`
151-
//
152-
// Then we will generate replace ranges for both
153-
// the `#[cfg(FALSE)] field: bool` and the entire
154-
// `#[cfg(FALSE)] struct Foo { #[cfg(FALSE)] field: bool }`
155-
//
156-
// By starting processing from the replace range with the greatest
157-
// start position, we ensure that any (outer) replace range which
158-
// encloses another (inner) replace range will fully overwrite the
159-
// inner range's replacement.
160-
for (node_range, target) in node_replacements.into_iter().rev() {
146+
// Process the replace ranges.
147+
for (node_range, target) in node_replacements.into_iter() {
161148
assert!(
162149
!node_range.0.is_empty(),
163150
"Cannot replace an empty node range: {:?}",
@@ -364,10 +351,9 @@ impl<'a> Parser<'a> {
364351
// from `ParserRange` form to `NodeRange` form. We will perform the actual
365352
// replacement only when we convert the `LazyAttrTokenStream` to an
366353
// `AttrTokenStream`.
367-
self.capture_state.parser_replacements
368-
[parser_replacements_start..parser_replacements_end]
369-
.iter()
370-
.cloned()
354+
self.capture_state
355+
.parser_replacements
356+
.drain(parser_replacements_start..parser_replacements_end)
371357
.chain(inner_attr_parser_replacements.into_iter())
372358
.map(|(parser_range, data)| {
373359
(NodeRange::new(parser_range, collect_pos.start_pos), data)

0 commit comments

Comments
 (0)