Skip to content

Rework how the disallowed qualifier in function type diagnostics are generated #142302

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 2 commits into from
Jun 14, 2025

Conversation

JonathanBrouwer
Copy link
Contributor

@JonathanBrouwer JonathanBrouwer commented Jun 10, 2025

This pull request fixes two independent issues:

  1. When qualifiers of a function type ptr are in the wrong order and one of them is async/const (not permitted on function types), the diagnostic suggests removing the incorrect qualifier. Fixes invalid token removal suggested on unsafe const fn() type #142268, which is an issue created by Trim extra whitespace in fn ptr suggestion span #133151. This is fixed by moving the check into parse_fn_front_matter, where better span information is available to generate the right suggestions.
  2. When qualifiers of a function type ptr are in the wrong order and one of them is async/const (not permitted on function types), cargo fix crashes because "cannot replace slice of data that was already replaced". This is fixed by not generating a suggestion for the "wrong order" diagnostic if the "disallowed qualifier" diagnostic is triggered.

There is a commit with failing tests so the test diff is clearer
r? @jdonszelmann

@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 Jun 10, 2025
@JonathanBrouwer JonathanBrouwer changed the title Invalid const token Rework how the disallowed qualifier lints are generated Jun 10, 2025
@JonathanBrouwer JonathanBrouwer changed the title Rework how the disallowed qualifier lints are generated Rework how the disallowed qualifier in function type lints are generated Jun 10, 2025
@JonathanBrouwer JonathanBrouwer changed the title Rework how the disallowed qualifier in function type lints are generated Rework how the disallowed qualifier in function type diagnostics are generated Jun 10, 2025
@jdonszelmann
Copy link
Contributor

@rustbot author

@rustbot rustbot 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 Jun 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 12, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@JonathanBrouwer JonathanBrouwer force-pushed the invalid-const-token branch 2 times, most recently from f4135a4 to 8cfeeb8 Compare June 13, 2025 10:33
@JonathanBrouwer
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 13, 2025

parse_fn_pointer_cannot_be_const = an `fn` pointer type cannot be `const`
.label = `const` because of this
.suggestion = remove the `const` qualifier
.note = allowed qualifiers are: `unsafe`, `extern`
Copy link
Contributor

Choose a reason for hiding this comment

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

could you say

unsafe and extern
?

Copy link
Contributor Author

@JonathanBrouwer JonathanBrouwer Jun 13, 2025

Choose a reason for hiding this comment

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

Fixed.
I decided to keep the : in the allowed qualifiers are: 'unsafe' and 'extern', but if you want to remove that lmk

@jdonszelmann
Copy link
Contributor

r=me after that

@jdonszelmann
Copy link
Contributor

@rustbot author

@rustbot rustbot 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 Jun 13, 2025
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
@JonathanBrouwer
Copy link
Contributor Author

@rustbot ready
Fixed comment & rebased on master

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 13, 2025
@jdonszelmann
Copy link
Contributor

bors-r-plus

@bors
Copy link
Collaborator

bors commented Jun 13, 2025

📌 Commit b131b6f has been approved by jdonszelmann

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 Jun 13, 2025
bors added a commit that referenced this pull request Jun 14, 2025
Rollup of 16 pull requests

Successful merges:

 - #140969 (Allow initializing logger with additional tracing Layer)
 - #141352 (builtin dyn impl no guide inference)
 - #142046 (add Vec::peek_mut)
 - #142273 (tests: Minicore `extern "gpu-kernel"` feature test)
 - #142302 (Rework how the disallowed qualifier in function type diagnostics are generated)
 - #142405 (Don't hardcode the intrinsic return types twice in the compiler)
 - #142434 ( Pre-install JS dependencies in tidy Dockerfile)
 - #142439 (doc: mention that intrinsics should not be called in user code)
 - #142441 (Delay replacing escaping bound vars in `FindParamInClause`)
 - #142449 (Require generic params for const generic params)
 - #142452 (Remove "intermittent" wording from `ReadDir`)
 - #142459 (Remove output helper bootstrap)
 - #142460 (cleanup search graph impl)
 - #142461 (compiletest: Clarify that `--no-capture` is needed with `--verbose`)
 - #142475 (Add platform support docs & maintainers for *-windows-msvc)
 - #142480 (tests: Convert two handwritten minicores to add-core-stubs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4db9554 into rust-lang:master Jun 14, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 14, 2025
rust-timer added a commit that referenced this pull request Jun 14, 2025
Rollup merge of #142302 - JonathanBrouwer:invalid-const-token, r=jdonszelmann

Rework how the disallowed qualifier in function type diagnostics are generated

This pull request fixes two independent issues:
1. When qualifiers of a function type ptr are in the wrong order and one of them is async/const (not permitted on function types), the diagnostic suggests removing the incorrect qualifier.  Fixes #142268, which is an issue created by #133151. This is fixed by moving the check into `parse_fn_front_matter`, where better span information is available to generate the right suggestions.
2. When qualifiers of a function type ptr are in the wrong order and one of them is async/const (not permitted on function types), `cargo fix` crashes because "cannot replace slice of data that was already replaced". This is fixed by not generating a suggestion for the "wrong order" diagnostic if the "disallowed qualifier" diagnostic is triggered.

There is a commit with failing tests so the test diff is clearer
r? `@jdonszelmann`
# 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.

invalid token removal suggested on unsafe const fn() type
4 participants