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>
cuviper · Jul 19, 2023 · 99b1897 · 99b1897
1 parent f53cef3
commit 99b1897
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 35 deletions.
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -267,8 +267,6 @@ lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead
 lint_improper_ctypes_char_reason = the `char` type has no C equivalent
 lint_improper_ctypes_dyn = trait objects have no C equivalent
 
-lint_improper_ctypes_enum_phantomdata = this enum contains a PhantomData field
-
 lint_improper_ctypes_enum_repr_help =
     consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
 

diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs
@@ -1009,39 +1009,36 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     ) -> FfiResult<'tcx> {
         use FfiResult::*;
 
-        let transparent_safety = def.repr().transparent().then(|| {
-            // Can assume that at most one field is not a ZST, so only check
-            // that field's type for FFI-safety.
+        let transparent_with_all_zst_fields = if def.repr().transparent() {
+            // Transparent newtypes have at most one non-ZST field which needs to be checked..
             if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
                 return self.check_field_type_for_ffi(cache, field, args);
-            } else {
-                // All fields are ZSTs; this means that the type should behave
-                // like (), which is FFI-unsafe... except if all fields are PhantomData,
-                // which is tested for below
-                FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None }
             }
-        });
-        // We can't completely trust repr(C) markings; make sure the fields are
-        // actually safe.
+
+            // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all
+            // `PhantomData`).
+            true
+        } else {
+            false
+        };
+
+        // We can't completely trust `repr(C)` markings, so make sure the fields are actually safe.
         let mut all_phantom = !variant.fields.is_empty();
         for field in &variant.fields {
-            match self.check_field_type_for_ffi(cache, &field, args) {
-                FfiSafe => {
-                    all_phantom = false;
-                }
-                FfiPhantom(..) if !def.repr().transparent() && def.is_enum() => {
-                    return FfiUnsafe {
-                        ty,
-                        reason: fluent::lint_improper_ctypes_enum_phantomdata,
-                        help: None,
-                    };
-                }
-                FfiPhantom(..) => {}
-                r => return transparent_safety.unwrap_or(r),
+            all_phantom &= match self.check_field_type_for_ffi(cache, &field, args) {
+                FfiSafe => false,
+                FfiPhantom(..) => true,
+                r @ FfiUnsafe { .. } => return r,
             }
         }
 
-        if all_phantom { FfiPhantom(ty) } else { transparent_safety.unwrap_or(FfiSafe) }
+        if all_phantom {
+            FfiPhantom(ty)
+        } else if transparent_with_all_zst_fields {
+            FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None }
+        } else {
+            FfiSafe
+        }
     }
 
     /// Checks if the given type is "ffi-safe" (has a stable, well-defined

diff --git a/tests/ui/repr/repr-transparent-issue-87496.rs b/tests/ui/repr/repr-transparent-issue-87496.rs
@@ -6,7 +6,7 @@
 struct TransparentCustomZst(());
 extern "C" {
     fn good17(p: TransparentCustomZst);
-    //~^ WARNING: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe
+    //~^ WARNING: `extern` block uses type `()`, which is not FFI-safe
 }
 
 fn main() {}
diff --git a/tests/ui/repr/repr-transparent-issue-87496.stderr b/tests/ui/repr/repr-transparent-issue-87496.stderr
@@ -1,15 +1,11 @@
-warning: `extern` block uses type `TransparentCustomZst`, which is not FFI-safe
+warning: `extern` block uses type `()`, which is not FFI-safe
   --> $DIR/repr-transparent-issue-87496.rs:8:18
    |
 LL |     fn good17(p: TransparentCustomZst);
    |                  ^^^^^^^^^^^^^^^^^^^^ not FFI-safe
    |
-   = note: this struct contains only zero-sized fields
-note: the type is defined here
-  --> $DIR/repr-transparent-issue-87496.rs:6:1
-   |
-LL | struct TransparentCustomZst(());
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: consider using a struct instead
+   = note: tuples have unspecified layout
    = note: `#[warn(improper_ctypes)]` on by default
 
 warning: 1 warning emitted