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.

Sign up for GitHub

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

Make #[repr(Rust)] incompatible with other (non-modifier) representation hints like C and simd #116829

Merged
merged 1 commit into from
Oct 19, 2023
Merged

Make #[repr(Rust)] incompatible with other (non-modifier) representation hints like C and simd #116829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions compiler/rustc_passes/src/check_attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,7 @@ impl CheckAttrVisitor<'_> {
.collect();

let mut int_reprs = 0;
let mut is_explicit_rust = false;
let mut is_c = false;
let mut is_simd = false;
let mut is_transparent = false;
Expand All @@ -1778,7 +1779,9 @@ impl CheckAttrVisitor<'_> {
}

match hint.name_or_empty() {
sym::Rust => {}
sym::Rust => {
is_explicit_rust = true;
}
sym::C => {
is_c = true;
match target {
Expand Down Expand Up @@ -1888,12 +1891,16 @@ impl CheckAttrVisitor<'_> {

// Error on repr(transparent, <anything else>).
if is_transparent && hints.len() > 1 {
let hint_spans: Vec<_> = hint_spans.clone().collect();
let hint_spans = hint_spans.clone().collect();
self.tcx.sess.emit_err(errors::TransparentIncompatible {
hint_spans,
target: target.to_string(),
});
}
if is_explicit_rust && (int_reprs > 0 || is_c || is_simd) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also turn the bools into a bitfield and check if the difference between the set bitflags and {Rust, align, packed} is empty to address “but not necessarily limited to” part of

We should ban all other repr(Rust, other) combinations, including, but not necesseraly limited to

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think refactoring this function would be a lot of good, as the current code seems to be error-prone and confusing. But obv this refactoring can come as a follow-up, which does not need a backport. (cc me if you'll do it please)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the things that would be nice is to default to disallowing, so that no changes can accidentally allow new combinations.

let hint_spans = hint_spans.clone().collect();
self.tcx.sess.emit_err(errors::ReprConflicting { hint_spans });
}
// Warn on repr(u8, u16), repr(C, simd), and c-like-enum-repr(C, u8)
if (int_reprs > 1)
|| (is_simd && is_c)
fmease marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -1910,7 +1917,7 @@ impl CheckAttrVisitor<'_> {
CONFLICTING_REPR_HINTS,
hir_id,
hint_spans.collect::<Vec<Span>>(),
errors::ReprConflicting,
errors::ReprConflictingLint,
);
}
}
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_passes/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,16 @@ pub struct ReprIdent {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_repr_conflicting, code = "E0566")]
pub struct ReprConflicting {
#[primary_span]
pub hint_spans: Vec<Span>,
}

#[derive(LintDiagnostic)]
#[diag(passes_repr_conflicting, code = "E0566")]
pub struct ReprConflicting;
pub struct ReprConflictingLint;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're interested in a follow-up, it's probably time to bump this lint up to a hard-error 😸


#[derive(Diagnostic)]
#[diag(passes_used_static)]
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/repr/explicit-rust-repr-conflicts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#[repr(C, Rust)] //~ ERROR conflicting representation hints
struct S {
a: i32,
}


#[repr(Rust)] //~ ERROR conflicting representation hints
#[repr(C)]
struct T {
a: i32,
}

#[repr(Rust, u64)] //~ ERROR conflicting representation hints
enum U {
V,
}

#[repr(Rust, simd)]
//~^ ERROR conflicting representation hints
//~| ERROR SIMD types are experimental and possibly buggy
struct F32x4(f32, f32, f32, f32);

fn main() {}
39 changes: 39 additions & 0 deletions tests/ui/repr/explicit-rust-repr-conflicts.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0658]: SIMD types are experimental and possibly buggy
--> $DIR/explicit-rust-repr-conflicts.rs:18:1
|
LL | #[repr(Rust, simd)]
| ^^^^^^^^^^^^^^^^^^^
|
= note: see issue #27731 <https://github.com/rust-lang/rust/issues/27731> for more information
= help: add `#![feature(repr_simd)]` to the crate attributes to enable

error[E0566]: conflicting representation hints
--> $DIR/explicit-rust-repr-conflicts.rs:1:8
|
LL | #[repr(C, Rust)]
| ^ ^^^^

error[E0566]: conflicting representation hints
--> $DIR/explicit-rust-repr-conflicts.rs:7:8
|
LL | #[repr(Rust)]
| ^^^^
LL | #[repr(C)]
| ^

error[E0566]: conflicting representation hints
--> $DIR/explicit-rust-repr-conflicts.rs:13:8
|
LL | #[repr(Rust, u64)]
| ^^^^ ^^^

error[E0566]: conflicting representation hints
--> $DIR/explicit-rust-repr-conflicts.rs:18:8
|
LL | #[repr(Rust, simd)]
| ^^^^ ^^^^

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0566, E0658.
For more information about an error, try `rustc --explain E0566`.
Loading