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

repr attributes can be applied at module level #64734

Closed
gnzlbg opened this issue Sep 24, 2019 · 9 comments
Closed

repr attributes can be applied at module level #64734

gnzlbg opened this issue Sep 24, 2019 · 9 comments
Labels
A-attributes Area: #[attributes(..)] C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 24, 2019

Spot what's wrong with this code:

#![repr(C)] struct Foo(i32);

extern "C" {
    fn foo(x: Foo);
}

fn main() {}

(Playground)

For some reason, top-level #![repr(..)] attributes are allowed, so Foo isn't repr(C). Sadly,
@rust-lang/underhanded isn't a thing.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Sep 24, 2019

cc @Centril ^^^ I don't know who is familiar with libsyntax or the library that handles this.

@jonas-schievink jonas-schievink added A-attributes Area: #[attributes(..)] C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 24, 2019
@jonas-schievink
Copy link
Contributor

I don't think attributes that affect codegen are checked for validity at all. The same thing happens for #[cold], #[inline], etc.

@hanna-kruppe
Copy link
Contributor

Some attributes including #[repr] and #[inline] are checked, it appears it's just not visiting the root module. Nested modules do get checked.

@Centril
Copy link
Contributor

Centril commented Sep 24, 2019

In case it turns out that many people rely on this, we could give reasonable (and arguably desirable) interpretation to #![inline] and #![repr(C)]: apply these recursively in the module to e.g. structs and fns (unless there is conflict, in which case the most-local attribute is chosen).

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Sep 24, 2019

If people rely on this compiling, but doing nothing, changing the repr of all types silently feels like a footgun.

@carbotaniuman
Copy link
Contributor

carbotaniuman commented Sep 25, 2019

I feel the standard warn by default, deny by default, hard error approach is the easiest way to handle this.

Maybe even skip to deny by default.

@varkor
Copy link
Member

varkor commented Sep 26, 2019

Given that occurrences of #![repr(C)] currently do nothing, and it's easy to search for in a codebase (and thus remove or audit), I don't see a strong reason not to start emitting errors, rather than just linting, on these, as long as we can determine this won't break downstream code if there's a crate making this mistake.

@varkor
Copy link
Member

varkor commented Sep 26, 2019

I don't think attributes that affect codegen are checked for validity at all. The same thing happens for #[cold], #[inline], etc.

We should fix this too, in the same manner as @rkruppe pointed out. (Either as part of this issue, or in a new issue.) Lack of validity checking can cause concrete problems, as in #64768.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Oct 5, 2020
…l, r=matthewjasper

Add check for doc alias attribute at crate level

Fixes rust-lang#76298, rust-lang#64734, rust-lang#69365.

r? @ollie27
@ehuss
Copy link
Contributor

ehuss commented May 14, 2021

Closing, as this is now a hard error via #76329.

@ehuss ehuss closed this as completed May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] 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

No branches or pull requests

7 participants