Skip to content

Fix overlapping spans in removing extra arguments - #108239

Merged
bors merged 1 commit into
rust-lang:masterfrom
clubby789:overlapping-spans
Feb 22, 2023
Merged

Fix overlapping spans in removing extra arguments#108239
bors merged 1 commit into
rust-lang:masterfrom
clubby789:overlapping-spans

Conversation

@clubby789

@clubby789clubby789 commented Feb 19, 2023

Copy link
Copy Markdown
Contributor

Fixes#108225

Each span is already extended to include the previous comma, so extending to the next comma is unecessary and causes an ICE with assertions on.

@rustbot label +A-diagnostics

@rustbot

rustbot commented Feb 19, 2023

Copy link
Copy Markdown
Collaborator

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

@rustbotrustbot 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. A-diagnostics Area: Messages for errors, warnings, and lints labels Feb 19, 2023
lukas-code
lukas-code reviewed Feb 19, 2023
View reviewed changes
Comment threadcompiler/rustc_hir_typeck/src/fn_ctxt/checks.rs Outdated
This was referenced Feb 19, 2023
Closed
Merged
compiler-errors
compiler-errors reviewed Feb 19, 2023
View reviewed changes
Comment threadtests/ui/argument-suggestions/extra_arguments.stderr Outdated
@clubby789

clubby789 commented Feb 19, 2023

Copy link
Copy Markdown
ContributorAuthor

I've added a new commit which merges contiguous Error::Extra into a single label/suggestion. I'm not sure if the resulting diagnostics are better as there's less noise, or worse as the individual types aren't reported.
This also fixes #108242

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors

compiler-errors commented Feb 20, 2023

Copy link
Copy Markdown
Contributor

I've added a new commit which merges contiguous Error::Extra into a single label/suggestion. I'm not sure if the resulting diagnostics are better as there's less noise, or worse as the individual types aren't reported.

Hm, I'm not sure if I have a strong opinion about it, though I think that change somewhat obscures the declared purpose of the PR in the title.

@clubby789

clubby789 commented Feb 20, 2023

Copy link
Copy Markdown
ContributorAuthor

Given #108244 seems to fix this without regressing other stuff I'll just revert back to the first commit

@clubby789
clubby789force-pushed the overlapping-spans branch 2 times, most recently from 45a313a to 3779c59CompareFebruary 20, 2023 18:36
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors

compiler-errors commented Feb 22, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

This code is kinda a mess (not your fault, pre-existing), but whatever. It could probably use some refactoring, but I won't let that block this PR landing.

@bors r+ rollup

@bors

bors commented Feb 22, 2023

Copy link
Copy Markdown
Collaborator

📌 Commit 0b9a3e2 has been approved by compiler-errors

It is now in the queue for this repository.

@borsbors 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 Feb 22, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2023
@matthiaskrgr
Rollup merge ofrust-lang#108239- clubby789:overlapping-spans, r=com…
2199207
…piler-errors
Fix overlapping spans in removing extra arguments
Fixesrust-lang#108225
Each span is already extended to include the previous comma, so extending to the *next* comma is unecessary and causes an ICE with assertions on.
`@rustbot` label +A-diagnostics
@matthiaskrgrmatthiaskrgr mentioned this pull request Feb 22, 2023
Closed
@GuillaumeGomezGuillaumeGomez mentioned this pull request Feb 22, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2023
@bors
Auto merge ofrust-lang#108339- GuillaumeGomez:rollup-4z02kas, r=Gui…
3b4d6e0
…llaumeGomez
Rollup of 8 pull requests
Successful merges:
- rust-lang#108110 (Move some `InferCtxt` methods to `EvalCtxt` in new solver)
- rust-lang#108168 (Fix ICE on type alias in recursion)
- rust-lang#108230 (Convert a hard-warning about named static lifetimes into lint "unused_lifetimes")
- rust-lang#108239 (Fix overlapping spans in removing extra arguments)
- rust-lang#108246 (Add an InstCombine for redundant casts)
- rust-lang#108264 (no-fail-fast support for tool testsuites)
- rust-lang#108310 (rustdoc: Fix duplicated attributes for first reexport)
- rust-lang#108318 (Remove unused FileDesc::get_cloexec)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 437f210 into rust-lang:masterFeb 22, 2023
@rustbotrustbot added this to the 1.69.0 milestone Feb 22, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2023
@matthiaskrgr
Rollup merge ofrust-lang#108244- lukas-code:semicolon-recovery-span…
5b64847
…, r=compiler-errors
Use span of semicolon for eager recovery in expression
Instead of the span of the token after the semicolon. This will hopefully cause fewer errors from overlapping spans.
fixesrust-lang#108242
based on rust-lang#108239
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2023
@matthiaskrgr
Rollup merge ofrust-lang#108244- lukas-code:semicolon-recovery-span…
a43380f
…, r=compiler-errors
Use span of semicolon for eager recovery in expression
Instead of the span of the token after the semicolon. This will hopefully cause fewer errors from overlapping spans.
fixesrust-lang#108242
based on rust-lang#108239
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@compiler-errorscompiler-errorscompiler-errors left review comments
@lukas-codelukas-codelukas-code left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@jackh726jackh726

Labels

A-diagnosticsArea: Messages for errors, warnings, and lintsS-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.69.0

Development

Successfully merging this pull request may close these issues.

ICE: (debug assertions) suggestion must not have overlapping part

7 participants

@clubby789@rustbot@rust-log-analyzer@compiler-errors@bors@lukas-code@jackh726