Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Confusing diagnostic for inversion of extern "..." and unsafe in function decl #87217

Closed
poliorcetics opened this issue Jul 17, 2021 · 2 comments · Fixed by #87235
Closed

Confusing diagnostic for inversion of extern "..." and unsafe in function decl #87217

poliorcetics opened this issue Jul 17, 2021 · 2 comments · Fixed by #87235
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@poliorcetics
Copy link
Contributor

poliorcetics commented Jul 17, 2021

Given the following code (playground link):

extern "C" unsafe fn test() {}

The current output is:

error: expected `{`, found keyword `unsafe`
 --> src/lib.rs:1:12
  |
1 | extern "C" unsafe fn test() {}
  |            ^^^^^^ expected `{`

error: could not compile `playground` due to previous error

Ideally the output should look like the one for inverting unsafe and a visibility like pub:

error: expected one of `extern` or `fn`, found keyword `pub`
 --> src/main.rs:1:8
  |
1 | unsafe pub fn test() {}
  | -------^^^
  | |      |
  | |      expected one of `extern` or `fn`
  | help: visibility `pub` must come before `unsafe`: `pub unsafe`

error: could not compile `playground` due to previous error

Meta rustc -V:

rustc 1.55.0-nightly (74ef0c3e4 2021-07-16)
@poliorcetics poliorcetics added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 17, 2021
@poliorcetics
Copy link
Contributor Author

Addition: this works with visibilities coming after extern "..." too:

error: expected `{`, found keyword `pub`
 --> src/main.rs:1:12
  |
1 | extern "C" pub fn test() {}
  |            ^^^ expected `{`

error: could not compile `playground` due to previous error

@poliorcetics
Copy link
Contributor Author

@rustbot claim

@bors bors closed this as completed in 74a11c6 Aug 9, 2021
tgross35 added a commit to tgross35/rust that referenced this issue Jul 26, 2024
…compiler-errors

Improve `extern "<abi>" unsafe fn()` error message

These errors were already reported in rust-lang#87217, and fixed by rust-lang#87235 but missed the case of an explicit ABI.

This PR does not cover multiple keywords like `extern "C" pub const unsafe fn()`, but I don't know what a good way to cover this  would be. It also seems rarer than `extern "C" unsafe` which I saw happen a few times in workshops.
tgross35 added a commit to tgross35/rust that referenced this issue Jul 26, 2024
…compiler-errors

Improve `extern "<abi>" unsafe fn()` error message

These errors were already reported in rust-lang#87217, and fixed by rust-lang#87235 but missed the case of an explicit ABI.

This PR does not cover multiple keywords like `extern "C" pub const unsafe fn()`, but I don't know what a good way to cover this  would be. It also seems rarer than `extern "C" unsafe` which I saw happen a few times in workshops.
tgross35 added a commit to tgross35/rust that referenced this issue Jul 26, 2024
…compiler-errors

Improve `extern "<abi>" unsafe fn()` error message

These errors were already reported in rust-lang#87217, and fixed by rust-lang#87235 but missed the case of an explicit ABI.

This PR does not cover multiple keywords like `extern "C" pub const unsafe fn()`, but I don't know what a good way to cover this  would be. It also seems rarer than `extern "C" unsafe` which I saw happen a few times in workshops.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 27, 2024
Rollup merge of rust-lang#128229 - tdittr:unsafe-extern-abi-error, r=compiler-errors

Improve `extern "<abi>" unsafe fn()` error message

These errors were already reported in rust-lang#87217, and fixed by rust-lang#87235 but missed the case of an explicit ABI.

This PR does not cover multiple keywords like `extern "C" pub const unsafe fn()`, but I don't know what a good way to cover this  would be. It also seems rarer than `extern "C" unsafe` which I saw happen a few times in workshops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant