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

Closure #[target_feature] only checked during codegen #70307

Closed
hanna-kruppe opened this issue Mar 23, 2020 · 1 comment · Fixed by #71205
Closed

Closure #[target_feature] only checked during codegen #70307

hanna-kruppe opened this issue Mar 23, 2020 · 1 comment · Fixed by #71205
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-closures Area: Closures (`|…| { … }`) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hanna-kruppe
Copy link
Contributor

I tried this code:

#![feature(stmt_expr_attributes)]

pub fn foo() {
    let f = #[target_feature(enable="magic")] || {};
    // f();
}

I expected to see this happen: error because the target feature does not exist and the closure is not unsafe.

Instead, this happened: the errors does not happen unless the closure is called in foo and foo is codegen'd. That is, I can get the expected errors by uncommenting the f(); line and running e.g. cargo build. but cargo check / rustc --emit metadata doesn't give any error, nor does cargo build give errors if foo is not codegen'd for other reasons (e.g., not pub in a library, or #[inline]).

Meta

rustc --version --verbose:

rustc 1.44.0-nightly (f509b26a7 2020-03-18)
binary: rustc
commit-hash: f509b26a7730d721ef87423a72b3fdf8724b4afa
commit-date: 2020-03-18
host: x86_64-unknown-linux-gnu
release: 1.44.0-nightly
LLVM version: 9.0
@hanna-kruppe hanna-kruppe added the C-bug Category: This is a bug. label Mar 23, 2020
@hanna-kruppe
Copy link
Contributor Author

I believe this is because rustc_passes::check_attr does not inspect target features on closures, as pointed out by @LeSeulArtichaut in #69274.

@jonas-schievink jonas-schievink added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-closures Area: Closures (`|…| { … }`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 23, 2020
RalfJung added a commit to RalfJung/rust that referenced this issue Apr 30, 2020
…vink

rustc: fix check_attr() for methods, closures and foreign functions

This fixes an issue that previously turned up for methods in rust-lang#69274, but also exists for closures and foreign function: `check_attr` does not call `codegen_fn_attrs()` for these types when it should, meaning that incorrectly used function attributes are not diagnosed without codegen.

The issue affects our UI tests, as they run with `--emit=metadata` by default, but as it turns out, this is not the only case: Function attributes are not checked on any dead code without this fix!

This makes the fix a **breaking change**. The following very silly Rust programs compiles fine on stable Rust when it should not, which is fixed by this PR.
```rust
fn main() {
    #[target_feature(enable = "sse2")]
    || {};
}
```

I assume any real-world program which may trigger this issue would at least emit a dead code warning, but of course that is no guarantee that such code does not exist...

Fixes rust-lang#70307
@bors bors closed this as completed in 4e6772b Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-closures Area: Closures (`|…| { … }`) C-bug Category: This is a bug. 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.

2 participants