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

#[target_feature] is allowed on default implementations #108646

Closed
Tracked by #69098
LeSeulArtichaut opened this issue Mar 2, 2023 · 2 comments · Fixed by #108983
Closed
Tracked by #69098

#[target_feature] is allowed on default implementations #108646

LeSeulArtichaut opened this issue Mar 2, 2023 · 2 comments · Fixed by #108983
Assignees
Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. F-target_feature_11 target feature 1.1 RFC requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Mar 2, 2023

The compiler currently allows safe default method implementations to be marked with #[target_feature]:

trait Foo {
    #[target_feature(enable = "avx2")] // no error
    fn foo() {}
}

which I don't think is allowed in RFC 2396. For reference, #[target_feature] is not allowed on trait implementations:

struct Bar;

impl Foo for Bar {
    #[target_feature(enable = "avx2")] // error: cannot be applied to safe trait method
    fn foo() {}
}

From my limited testing, this doesn't seem to be unsound however, as the compiler seems to consider all implementations of Foo::foo as having #[target_feature(enable = "avx2")].

cc #69098
@rustbot label T-lang T-compiler C-bug F-target_feature_11

@rustbot rustbot added C-bug Category: This is a bug. F-target_feature_11 target feature 1.1 RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Mar 2, 2023
@est31
Copy link
Member

est31 commented Mar 2, 2023

A good resolution to this would be to move it into a separate feature gate I guess?

@LeSeulArtichaut
Copy link
Contributor Author

@rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 2, 2023
…iler-errors

Revert stabilization of `#![feature(target_feature_11)]`

This reverts rust-lang#99767 due to the presence of bugs rust-lang#108645 and rust-lang#108646.

cc `@joshtriplett`
cc tracking issue rust-lang#69098
r? `@ghost`
@workingjubilee workingjubilee added requires-nightly This issue requires a nightly compiler in some way. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. labels Mar 3, 2023
@bors bors closed this as completed in f74bb35 Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. C-bug Category: This is a bug. F-target_feature_11 target feature 1.1 RFC requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants