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

diagnostics improvements for invalid target_feature error #109531

Open
est31 opened this issue Mar 23, 2023 · 1 comment
Open

diagnostics improvements for invalid target_feature error #109531

est31 opened this issue Mar 23, 2023 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@est31
Copy link
Member

est31 commented Mar 23, 2023

When a target feature is found that doesn't exist on the current target but does exist on another target, it should be explicitly pointed out in the error message. Copying the example from this comment:

#[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
pub unsafe fn foo() {}

When you compile it on M1/M2 you get:

error: the feature named `sse2` is not valid for this target
  --> $DIR/issue-108680.rs:3:18
   |
LL | #[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
   |                  ^^^^^^^^^^^^^^^ `sse2` is not valid for this target

error: the feature named `avx` is not valid for this target
  --> $DIR/issue-108680.rs:3:35
   |
LL | #[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
   |                                   ^^^^^^^^^^^^^^ `avx` is not valid for this target

error: the feature named `sse` is not valid for this target
  --> $DIR/issue-108680.rs:3:51
   |
LL | #[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
   |                                                   ^^^^^^^^^^^^^^ `sse` is not valid for this target

Ideally this would mention that these target features are valid for the arm64 target architecture/family:

error: the feature named `sse2` is not valid for this target
  --> $DIR/issue-108680.rs:3:18
   |
LL | #[target_feature(enable = "sse2", enable = "avx", enable = "sse")]
   |                  ^^^^^^^^^^^^^^^ `sse2` is not valid for this target
   = note: `sse2` is present on the `x86_64` target architecture. Did you mean to compile for that target, or use conditional compilation?

The printing also ideally would support features present on multiple target architectures, for example the aes feature is present on both arm and x86:

// only-wasm32
#[target_feature(enable = "aes")]
pub unsafe fn foo() {}

For aarch64 vs arm it might make sense to always mention them both if the feature is present on both, as the two languages are way different from each other (while x86 is very backwards compatible).

As a second thing, it would be nice if it could provide typo suggestions:

#[target_feature(enable = "avc")]
pub unsafe fn foo() {}
error: the feature named `avc` is not valid for this target
 --> src/main.rs:1:18
  |
1 | #[target_feature(enable = "avc")]
  |                  ^^^^^^^^^^^^^^ `avc` is not valid for this target
  = help: a similar feature `avx` exists for this target

But I'm less sure if this is a good idea as target features are often very short and similar to each other. Suggesting the wrong target feature might not be detected as easily.

@rustbot label A-diagnostics A-target-feature

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. labels Mar 23, 2023
@est31
Copy link
Member Author

est31 commented Mar 23, 2023

cc @rohaquinlop as you indicated you wanted to work on this

@Nilstrieb Nilstrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
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 A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants