Skip to content

Remove unnecessary range replacements #128224

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Jul 27, 2024

Conversation

nnethercote
Copy link
Contributor

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

r? @petrochenkov

The current code is this:
```
self.capture_state.replace_ranges.push((start_pos..end_pos, Some(target)));
self.capture_state.replace_ranges.extend(inner_attr_replace_ranges);
```
What's not obvious is that every range in `inner_attr_replace_ranges`
must be a strict sub-range of `start_pos..end_pos`. Which means, in
`LazyAttrTokenStreamImpl::to_attr_token_stream`, they will be done
first, and then the `start_pos..end_pos` replacement will just overwrite
them. So they aren't needed.
Imagine you have replace ranges (2..20,X) and (5..15,Y), and these tokens:
```
a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u,v,w,x
```
If we replace (5..15,Y) first, then (2..20,X) we get this sequence
```
a,b,c,d,e,Y,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x
a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x
```
which is what we want.

If we do it in the other order, we get this:
```
a,b,X,_,_,_,_,_,_,_,_,_,_,_,_,p,q,r,s,t,u,v,w,x
a,b,X,_,_,Y,_,_,_,_,_,_,_,_,_,_,_,_,_,_,u,v,w,x
```
which is wrong. So it's true that we need the `.rev()` but the comment
is wrong about why.
A fully imperative style is easier to read than a half-iterator,
half-imperative style. Also, rename `inner_attr` as `attr` because it
might be an outer attribute.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 26, 2024
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 26, 2024

📌 Commit 55d37ae has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 26, 2024
…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`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#121676 (Support ?Trait bounds in supertraits and dyn Trait under a feature gate)
 - rust-lang#124339 (allow overwriting the output of `rustc --version`)
 - rust-lang#128223 (Refactor complex conditions in `collect_tokens_trailing_token`)
 - rust-lang#128224 (Remove unnecessary range replacements)
 - rust-lang#128226 (Remove redundant option that was just encoding that a slice was empty)
 - rust-lang#128227 (CI: do not respect custom try jobs for unrolled perf builds)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 26, 2024
…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``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#121676 (Support ?Trait bounds in supertraits and dyn Trait under a feature gate)
 - rust-lang#124941 (Stabilize const `{integer}::from_str_radix` i.e. `const_int_from_str`)
 - rust-lang#128210 (rustdoc: change title of search results)
 - rust-lang#128223 (Refactor complex conditions in `collect_tokens_trailing_token`)
 - rust-lang#128224 (Remove unnecessary range replacements)
 - rust-lang#128226 (Remove redundant option that was just encoding that a slice was empty)
 - rust-lang#128227 (CI: do not respect custom try jobs for unrolled perf builds)
 - rust-lang#128229 (Improve `extern "<abi>" unsafe fn()` error message)
 - rust-lang#128235 (Fix `Iterator::filter` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 26, 2024
…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```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#124941 (Stabilize const `{integer}::from_str_radix` i.e. `const_int_from_str`)
 - rust-lang#127853 (`#[naked]`: report incompatible attributes)
 - rust-lang#128210 (rustdoc: change title of search results)
 - rust-lang#128223 (Refactor complex conditions in `collect_tokens_trailing_token`)
 - rust-lang#128224 (Remove unnecessary range replacements)
 - rust-lang#128226 (Remove redundant option that was just encoding that a slice was empty)
 - rust-lang#128227 (CI: do not respect custom try jobs for unrolled perf builds)
 - rust-lang#128229 (Improve `extern "<abi>" unsafe fn()` error message)
 - rust-lang#128235 (Fix `Iterator::filter` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#124941 (Stabilize const `{integer}::from_str_radix` i.e. `const_int_from_str`)
 - rust-lang#128201 (Implement `Copy`/`Clone` for async closures)
 - rust-lang#128210 (rustdoc: change title of search results)
 - rust-lang#128223 (Refactor complex conditions in `collect_tokens_trailing_token`)
 - rust-lang#128224 (Remove unnecessary range replacements)
 - rust-lang#128226 (Remove redundant option that was just encoding that a slice was empty)
 - rust-lang#128227 (CI: do not respect custom try jobs for unrolled perf builds)
 - rust-lang#128229 (Improve `extern "<abi>" unsafe fn()` error message)
 - rust-lang#128235 (Fix `Iterator::filter` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit af52be2 into rust-lang:master Jul 27, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Jul 27, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 27, 2024
Rollup merge of rust-lang#128224 - nnethercote:fewer-replace_ranges, 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````
@nnethercote nnethercote deleted the fewer-replace_ranges branch July 29, 2024 02:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants