Skip to content

Replace span_look_ahead with span_followed_by#154745

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
chenyukang:yukang-fix-span-api
Apr 8, 2026
Merged

Replace span_look_ahead with span_followed_by#154745
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
chenyukang:yukang-fix-span-api

Conversation

@chenyukang
Copy link
Copy Markdown
Member

@chenyukang chenyukang commented Apr 3, 2026

While reviewing that PR #154703 (comment), I found that magic number 100, let's remove it, and seems span_followed_by is a better name.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

Some changes occurred in match checking

cc @Nadrieril

@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 Apr 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 3, 2026

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 11 candidates

// In case this could be a struct literal that needs to be surrounded
// by parentheses, find the appropriate span.
let close_brace_span = sm.span_look_ahead(followed_brace_span, "}", Some(50));
let close_brace_span =
Copy link
Copy Markdown
Member Author

@chenyukang chenyukang Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can not use span_look_ahead here, because we only want to make sure there is a } in 50 range.

otherwise we can not handle the scenario like U { /* keep comment here */ }, since there are some non whitespace chars inside { }.

https://github.com/rust-lang/rust/pull/154745/changes#diff-68dd28577bfdbdba4a4c4ee9d20797f2f0e3939d926d4bdcbe13b1117a2dbe32R35-R38 is the test case for this.

Copy link
Copy Markdown
Member

@Kivooeo Kivooeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks reasonable to me, but I'd like someone else to take a look too

View changes since this review

let close_brace_span =
sm.span_to_next_source(open_brace_span).ok().and_then(|next_source| {
let offset = next_source.find('}')?;
if next_source[..offset].chars().count() >= 50 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining what this >= 50 check is doing, and why 50 was chosen.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

50 was a heuristic number trying to find pattern { ... } nearby.
I just reconsider it, seems it's better to have a simple check for the nest braces and also remove this magic number here (https://github.com/rust-lang/rust/compare/6fce979d7e01d25123417481b3c20dd389c63d88..b7e05132e8d10fb19e6a91afd937d7bc0e8d433b).

Anyway, we are on the error handling path, performance is not the highest priority.

@nnethercote nnethercote added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 6, 2026
@chenyukang chenyukang force-pushed the yukang-fix-span-api branch from 6fce979 to b7e0513 Compare April 8, 2026 01:16
@rust-log-analyzer

This comment has been minimized.

@chenyukang chenyukang force-pushed the yukang-fix-span-api branch from b7e0513 to ef9b7c2 Compare April 8, 2026 01:34
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 8, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@nnethercote
Copy link
Copy Markdown
Contributor

Looks good, thanks.

@bors r+ rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 8, 2026

📌 Commit ef9b7c2 has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 8, 2026
…nnethercote

Replace span_look_ahead with span_followed_by

While reviewing that PR rust-lang#154703 (comment), I found that magic number 100, let's remove it, and seems `span_followed_by` is a better name.
rust-bors Bot pushed a commit that referenced this pull request Apr 8, 2026
…uwer

Rollup of 15 pull requests

Successful merges:

 - #153995 (Use convergent attribute to funcs for GPU targets)
 - #154184 (stabilize s390x vector registers)
 - #151898 (constify DoubleEndedIterator)
 - #154235 (remove unnecessary variables and delimiter check)
 - #154473 (move borrow checker tests)
 - #154745 (Replace span_look_ahead with span_followed_by)
 - #154778 (make field representing types invariant over the base type)
 - #154867 (Fix private fields diagnostics and improve error messages)
 - #154879 (Don't store `pattern_ty` in `TestableCase`)
 - #154910 (Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`)
 - #154923 (Fix ICE in next-solver dyn-compatibility check)
 - #154934 (Add getters for `rustc_pattern_analysis::constructor::Slice` fields)
 - #154938 (match exhaustiveness: Show the guard exhaustivity note only when it's the guards alone that cause non-exhaustiveness)
 - #154961 (Use derived impl for `GappedRange` subdiagnostic)
 - #154980 (rustc-dev-guide subtree update)
@rust-bors rust-bors Bot merged commit 4a3978a into rust-lang:main Apr 8, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Apr 8, 2026
rust-timer added a commit that referenced this pull request Apr 8, 2026
Rollup merge of #154745 - chenyukang:yukang-fix-span-api, r=nnethercote

Replace span_look_ahead with span_followed_by

While reviewing that PR #154703 (comment), I found that magic number 100, let's remove it, and seems `span_followed_by` is a better name.
github-actions Bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 13, 2026
…uwer

Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#153995 (Use convergent attribute to funcs for GPU targets)
 - rust-lang/rust#154184 (stabilize s390x vector registers)
 - rust-lang/rust#151898 (constify DoubleEndedIterator)
 - rust-lang/rust#154235 (remove unnecessary variables and delimiter check)
 - rust-lang/rust#154473 (move borrow checker tests)
 - rust-lang/rust#154745 (Replace span_look_ahead with span_followed_by)
 - rust-lang/rust#154778 (make field representing types invariant over the base type)
 - rust-lang/rust#154867 (Fix private fields diagnostics and improve error messages)
 - rust-lang/rust#154879 (Don't store `pattern_ty` in `TestableCase`)
 - rust-lang/rust#154910 (Suppress `unreachable_code` lint in `derive(PartialEq, Clone)`)
 - rust-lang/rust#154923 (Fix ICE in next-solver dyn-compatibility check)
 - rust-lang/rust#154934 (Add getters for `rustc_pattern_analysis::constructor::Slice` fields)
 - rust-lang/rust#154938 (match exhaustiveness: Show the guard exhaustivity note only when it's the guards alone that cause non-exhaustiveness)
 - rust-lang/rust#154961 (Use derived impl for `GappedRange` subdiagnostic)
 - rust-lang/rust#154980 (rustc-dev-guide subtree update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

5 participants