Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit e9cd650

Browse files
davidtwcocuviper
authored andcommitted
lint: refactor check_variant_for_ffi
Simplify this function a bit, it was quite hard to reason about. Signed-off-by: David Wood <david@davidtw.co> (cherry picked from commit 99b1897)
1 parent 7744a86 commit e9cd650

File tree

4 files changed

+26
-35
lines changed

4 files changed

+26
-35
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead
262262
lint_improper_ctypes_char_reason = the `char` type has no C equivalent
263263
lint_improper_ctypes_dyn = trait objects have no C equivalent
264264
265-
lint_improper_ctypes_enum_phantomdata = this enum contains a PhantomData field
266-
267265
lint_improper_ctypes_enum_repr_help =
268266
consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
269267

compiler/rustc_lint/src/types.rs

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,39 +1006,36 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10061006
) -> FfiResult<'tcx> {
10071007
use FfiResult::*;
10081008

1009-
let transparent_safety = def.repr().transparent().then(|| {
1010-
// Can assume that at most one field is not a ZST, so only check
1011-
// that field's type for FFI-safety.
1009+
let transparent_with_all_zst_fields = if def.repr().transparent() {
1010+
// Transparent newtypes have at most one non-ZST field which needs to be checked..
10121011
if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
10131012
return self.check_field_type_for_ffi(cache, field, substs);
1014-
} else {
1015-
// All fields are ZSTs; this means that the type should behave
1016-
// like (), which is FFI-unsafe... except if all fields are PhantomData,
1017-
// which is tested for below
1018-
FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None }
10191013
}
1020-
});
1021-
// We can't completely trust repr(C) markings; make sure the fields are
1022-
// actually safe.
1014+
1015+
// ..or have only ZST fields, which is FFI-unsafe (unless those fields are all
1016+
// `PhantomData`).
1017+
true
1018+
} else {
1019+
false
1020+
};
1021+
1022+
// We can't completely trust `repr(C)` markings, so make sure the fields are actually safe.
10231023
let mut all_phantom = !variant.fields.is_empty();
10241024
for field in &variant.fields {
1025-
match self.check_field_type_for_ffi(cache, &field, substs) {
1026-
FfiSafe => {
1027-
all_phantom = false;
1028-
}
1029-
FfiPhantom(..) if !def.repr().transparent() && def.is_enum() => {
1030-
return FfiUnsafe {
1031-
ty,
1032-
reason: fluent::lint_improper_ctypes_enum_phantomdata,
1033-
help: None,
1034-
};
1035-
}
1036-
FfiPhantom(..) => {}
1037-
r => return transparent_safety.unwrap_or(r),
1025+
all_phantom &= match self.check_field_type_for_ffi(cache, &field, substs) {
1026+
FfiSafe => false,
1027+
FfiPhantom(..) => true,
1028+
r @ FfiUnsafe { .. } => return r,
10381029
}
10391030
}
10401031

1041-
if all_phantom { FfiPhantom(ty) } else { transparent_safety.unwrap_or(FfiSafe) }
1032+
if all_phantom {
1033+
FfiPhantom(ty)
1034+
} else if transparent_with_all_zst_fields {
1035+
FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None }
1036+
} else {
1037+
FfiSafe
1038+
}
10421039
}
10431040

10441041
/// Checks if the given type is "ffi-safe" (has a stable, well-defined

tests/ui/repr/repr-transparent-issue-87496.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
struct TransparentCustomZst(());
77
extern "C" {
88
fn good17(p: TransparentCustomZst);
9-
//~^ WARNING: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe
9+
//~^ WARNING: `extern` block uses type `()`, which is not FFI-safe
1010
}
1111

1212
fn main() {}

tests/ui/repr/repr-transparent-issue-87496.stderr

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
1-
warning: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe
1+
warning: `extern` block uses type `()`, which is not FFI-safe
22
--> $DIR/repr-transparent-issue-87496.rs:8:18
33
|
44
LL | fn good17(p: TransparentCustomZst);
55
| ^^^^^^^^^^^^^^^^^^^^ not FFI-safe
66
|
7-
= note: this struct contains only zero-sized fields
8-
note: the type is defined here
9-
--> $DIR/repr-transparent-issue-87496.rs:6:1
10-
|
11-
LL | struct TransparentCustomZst(());
12-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
7+
= help: consider using a struct instead
8+
= note: tuples have unspecified layout
139
= note: `#[warn(improper_ctypes)]` on by default
1410

1511
warning: 1 warning emitted

0 commit comments

Comments
 (0)