lint ImproperCTypes: misc. changes due to review

niacdoial · niacdoial · commit c414d4183c4e · 2025-08-07T21:57:01.000+02:00
- relaxed the uninhabited types allowed on function return
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -432,10 +432,7 @@ lint_improper_ctypes_tuple_help = consider using a struct instead
 lint_improper_ctypes_tuple_reason = tuples have unspecified layout
 
 lint_improper_ctypes_uninhabited_enum = zero-variant enums and other uninhabited types are not allowed in function arguments and static variables
-lint_improper_ctypes_uninhabited_enum_deep = zero-variant enums and other uninhabited types are only allowed in function returns if used directly
 lint_improper_ctypes_uninhabited_never = the never type (`!`) and other uninhabited types are not allowed in function arguments and static variables
-lint_improper_ctypes_uninhabited_never_deep = the never type (`!`) and other uninhabited types are only allowed in function returns if used directly
-lint_improper_ctypes_uninhabited_use_direct = if you meant to have a function that never returns, consider setting its return type to the never type (`!`) or a zero-variant enum
 
 lint_improper_ctypes_union_consider_transparent = `{$ty}` has exactly one non-zero-sized field, consider making it `#[repr(transparent)]` instead
 lint_improper_ctypes_union_fieldless_help = consider adding a member to this union
diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs
@@ -581,40 +581,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     }
 
     /// Checks whether an uninhabited type (one without valid values) is safe-ish to have here
-    fn visit_uninhabited(
-        &self,
-        state: CTypesVisitorState,
-        outer_ty: Option<Ty<'tcx>>,
-        ty: Ty<'tcx>,
-    ) -> FfiResult<'tcx> {
-        if state.is_in_function_return()
-            && matches!(outer_ty.map(|ty| ty.kind()), None | Some(ty::FnPtr(..)))
-        {
+    fn visit_uninhabited(&self, state: CTypesVisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> {
+        if state.is_in_function_return() {
             FfiResult::FfiSafe
         } else {
-            let help = if state.is_in_function_return() {
-                Some(fluent::lint_improper_ctypes_uninhabited_use_direct)
-            } else {
-                None
-            };
             let desc = match ty.kind() {
-                ty::Adt(..) => {
-                    if state.is_in_function_return() {
-                        fluent::lint_improper_ctypes_uninhabited_enum_deep
-                    } else {
-                        fluent::lint_improper_ctypes_uninhabited_enum
-                    }
-                }
-                ty::Never => {
-                    if state.is_in_function_return() {
-                        fluent::lint_improper_ctypes_uninhabited_never_deep
-                    } else {
-                        fluent::lint_improper_ctypes_uninhabited_never
-                    }
-                }
+                ty::Adt(..) => fluent::lint_improper_ctypes_uninhabited_enum,
+                ty::Never => fluent::lint_improper_ctypes_uninhabited_never,
                 r @ _ => bug!("unexpected ty_kind in uninhabited type handling: {:?}", r),
             };
-            FfiResult::new_with_reason(ty, desc, help)
+            FfiResult::new_with_reason(ty, desc, None)
         }
     }
 
@@ -623,14 +599,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     fn visit_numeric(&self, ty: Ty<'tcx>) -> FfiResult<'tcx> {
         // FIXME: for now, this is very incomplete, and seems to assume a x86_64 target
         match ty.kind() {
+            // note: before rust 1.77, 128-bit ints were not FFI-safe on x86_64
+            // ...they probably are still unsafe on i686 and other x86_32 architectures
             ty::Int(..) | ty::Uint(..) | ty::Float(..) => FfiResult::FfiSafe,
 
             ty::Char => FfiResult::new_with_reason(
                 ty,
                 fluent::lint_improper_ctypes_char_reason,
                 Some(fluent::lint_improper_ctypes_char_help),
             ),
-            _ => bug!("visit_numeric is to be called with numeric (int, float) types"),
+            _ => bug!("visit_numeric is to be called with numeric (char, int, float) types"),
         }
     }
 
@@ -767,7 +745,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                 // determine if there is 0 or 1 non-1ZST field, and which it is.
                 // (note: for enums, "transparent" means 1-variant)
                 if ty.is_privately_uninhabited(self.cx.tcx, self.cx.typing_env()) {
-                    // let's consider transparent structs are considered unsafe if uninhabited,
+                    // let's consider transparent structs to be maybe unsafe if uninhabited,
                     // even if that is because of fields otherwise ignored in FFI-safety checks
                     // FIXME: and also maybe this should be "!is_inhabited_from" but from where?
                     ffires_accumulator += variant
@@ -778,9 +756,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                             let mut field_res = self.visit_type(state, Some(ty), field_ty);
                             field_res.take_with_core_note(&[
                                 fluent::lint_improper_ctypes_uninhabited_enum,
-                                fluent::lint_improper_ctypes_uninhabited_enum_deep,
                                 fluent::lint_improper_ctypes_uninhabited_never,
-                                fluent::lint_improper_ctypes_uninhabited_never_deep,
                             ])
                         })
                         .reduce(|r1, r2| r1 + r2)
@@ -872,19 +848,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                 let help = if non_1zst_count == 1
                     && last_non_1zst.map(|field_i| fields_ok_list[field_i]) == Some(true)
                 {
-                    if ty.is_privately_uninhabited(self.cx.tcx, self.cx.typing_env()) {
-                        // uninhabited types can't be helped by being turned transparent
-                        None
-                    } else {
-                        match def.adt_kind() {
-                            AdtKind::Struct => {
-                                Some(fluent::lint_improper_ctypes_struct_consider_transparent)
-                            }
-                            AdtKind::Union => {
-                                Some(fluent::lint_improper_ctypes_union_consider_transparent)
-                            }
-                            AdtKind::Enum => bug!("cannot suggest an enum to be repr(transparent)"),
+                    match def.adt_kind() {
+                        AdtKind::Struct => {
+                            Some(fluent::lint_improper_ctypes_struct_consider_transparent)
+                        }
+                        AdtKind::Union => {
+                            Some(fluent::lint_improper_ctypes_union_consider_transparent)
                         }
+                        AdtKind::Enum => bug!("cannot suggest an enum to be repr(transparent)"),
                     }
                 } else {
                     None
@@ -978,7 +949,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     fn visit_enum(
         &self,
         state: CTypesVisitorState,
-        outer_ty: Option<Ty<'tcx>>,
         ty: Ty<'tcx>,
         def: AdtDef<'tcx>,
         args: GenericArgsRef<'tcx>,
@@ -988,7 +958,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
         if def.variants().is_empty() {
             // Empty enums are implicitely handled as the never type:
-            return self.visit_uninhabited(state, outer_ty, ty);
+            return self.visit_uninhabited(state, ty);
         }
         // Check for a repr() attribute to specify the size of the
         // discriminant.
@@ -1008,6 +978,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             );
         }
 
+        // FIXME: connect `def.repr().int` to visit_numeric
+        // (for now it's OK, `repr(char)` doesn't exist and visit_numeric doesn't warn on anything else)
+
         let non_exhaustive = def.variant_list_has_applicable_non_exhaustive();
         // Check the contained variants.
 
@@ -1042,9 +1015,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                     let mut variant_res = self.visit_variant_fields(state, ty, def, variant, args);
                     variants_uninhabited_ffires[variant_i] = variant_res.take_with_core_note(&[
                         fluent::lint_improper_ctypes_uninhabited_enum,
-                        fluent::lint_improper_ctypes_uninhabited_enum_deep,
                         fluent::lint_improper_ctypes_uninhabited_never,
-                        fluent::lint_improper_ctypes_uninhabited_never_deep,
                     ]);
                     // FIXME: check that enums allow any (up to all) variants to be phantoms?
                     // (previous code says no, but I don't know why? the problem with phantoms is that they're ZSTs, right?)
@@ -1102,7 +1073,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                         }
                         self.visit_struct_or_union(state, ty, def, args)
                     }
-                    AdtKind::Enum => self.visit_enum(state, outer_ty, ty, def, args),
+                    AdtKind::Enum => self.visit_enum(state, ty, def, args),
                 }
             }
 
@@ -1237,7 +1208,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
 
             ty::Foreign(..) => FfiSafe,
 
-            ty::Never => self.visit_uninhabited(state, outer_ty, ty),
+            ty::Never => self.visit_uninhabited(state, ty),
 
             // This is only half of the checking-for-opaque-aliases story:
             // since they are liable to vanish on normalisation, we need a specific to find them through
diff --git a/src/tools/clippy/tests/ui/ptr_arg.stderr b/src/tools/clippy/tests/ui/ptr_arg.stderr
@@ -220,13 +220,13 @@ LL |     fn cow_bad_ret_ty_2<'a, 'b>(input: &'a Cow<'a, str>) -> &'b str {
    |                                        ^^^^^^^^^^^^^^^^ help: change this to: `&str`
 
 error: writing `&String` instead of `&str` involves a new object where a slice will do
-  --> tests/ui/ptr_arg.rs:348:17
+  --> tests/ui/ptr_arg.rs:347:17
    |
 LL |     fn good(v1: &String, v2: &String) {
    |                 ^^^^^^^ help: change this to: `&str`
 
 error: writing `&String` instead of `&str` involves a new object where a slice will do
-  --> tests/ui/ptr_arg.rs:348:30
+  --> tests/ui/ptr_arg.rs:347:30
    |
 LL |     fn good(v1: &String, v2: &String) {
    |                              ^^^^^^^ help: change this to: `&str`
@@ -285,4 +285,3 @@ LL |     fn cow_good_ret_ty<'a>(input: &'a Cow<'a, str>) -> &'a str {
    |                                                         ++
 
 error: aborting due to 33 previous errors
-
diff --git a/tests/ui/lint/improper_ctypes/lint_uninhabited.rs b/tests/ui/lint/improper_ctypes/lint_uninhabited.rs
@@ -32,7 +32,7 @@ struct HalfHiddenUninhabited {
 extern "C" {
 
 fn bad_entry(e: AlsoUninhabited); //~ ERROR: uses type `AlsoUninhabited`
-fn bad_exit()->AlsoUninhabited; //~ ERROR: uses type `AlsoUninhabited`
+fn bad_exit()->AlsoUninhabited;
 
 fn bad0_entry(e: Uninhabited); //~ ERROR: uses type `Uninhabited`
 fn bad0_exit()->Uninhabited;
@@ -46,7 +46,7 @@ fn never_exit()->!;
 }
 
 extern "C" fn impl_bad_entry(e: AlsoUninhabited) {} //~ ERROR: uses type `AlsoUninhabited`
-extern "C" fn impl_bad_exit()->AlsoUninhabited { //~ ERROR: uses type `AlsoUninhabited`
+extern "C" fn impl_bad_exit()->AlsoUninhabited {
     AlsoUninhabited{
         a: impl_bad0_exit(),
         b: 0,
diff --git a/tests/ui/lint/improper_ctypes/lint_uninhabited.stderr b/tests/ui/lint/improper_ctypes/lint_uninhabited.stderr
@@ -4,6 +4,7 @@ error: `extern` block uses type `AlsoUninhabited`, which is not FFI-safe
 LL | fn bad_entry(e: AlsoUninhabited);
    |                 ^^^^^^^^^^^^^^^ not FFI-safe
    |
+   = help: `AlsoUninhabited` has exactly one non-zero-sized field, consider making it `#[repr(transparent)]` instead
    = note: this struct/enum/union (`AlsoUninhabited`) is FFI-unsafe due to a `Uninhabited` field
 note: the type is defined here
   --> $DIR/lint_uninhabited.rs:12:1
@@ -22,26 +23,6 @@ note: the lint level is defined here
 LL | #![deny(improper_ctypes)]
    |         ^^^^^^^^^^^^^^^
 
-error: `extern` block uses type `AlsoUninhabited`, which is not FFI-safe
-  --> $DIR/lint_uninhabited.rs:35:16
-   |
-LL | fn bad_exit()->AlsoUninhabited;
-   |                ^^^^^^^^^^^^^^^ not FFI-safe
-   |
-   = note: this struct/enum/union (`AlsoUninhabited`) is FFI-unsafe due to a `Uninhabited` field
-note: the type is defined here
-  --> $DIR/lint_uninhabited.rs:12:1
-   |
-LL | struct AlsoUninhabited{
-   | ^^^^^^^^^^^^^^^^^^^^^^
-   = help: if you meant to have a function that never returns, consider setting its return type to the never type (`!`) or a zero-variant enum
-   = note: zero-variant enums and other uninhabited types are only allowed in function returns if used directly
-note: the type is defined here
-  --> $DIR/lint_uninhabited.rs:9:1
-   |
-LL | enum Uninhabited{}
-   | ^^^^^^^^^^^^^^^^
-
 error: `extern` block uses type `Uninhabited`, which is not FFI-safe
   --> $DIR/lint_uninhabited.rs:37:18
    |
@@ -69,6 +50,7 @@ error: `extern` fn uses type `AlsoUninhabited`, which is not FFI-safe
 LL | extern "C" fn impl_bad_entry(e: AlsoUninhabited) {}
    |                                 ^^^^^^^^^^^^^^^ not FFI-safe
    |
+   = help: `AlsoUninhabited` has exactly one non-zero-sized field, consider making it `#[repr(transparent)]` instead
    = note: this struct/enum/union (`AlsoUninhabited`) is FFI-unsafe due to a `Uninhabited` field
 note: the type is defined here
   --> $DIR/lint_uninhabited.rs:12:1
@@ -87,26 +69,6 @@ note: the lint level is defined here
 LL | #![deny(improper_c_fn_definitions, improper_c_callbacks)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: `extern` fn uses type `AlsoUninhabited`, which is not FFI-safe
-  --> $DIR/lint_uninhabited.rs:49:32
-   |
-LL | extern "C" fn impl_bad_exit()->AlsoUninhabited {
-   |                                ^^^^^^^^^^^^^^^ not FFI-safe
-   |
-   = note: this struct/enum/union (`AlsoUninhabited`) is FFI-unsafe due to a `Uninhabited` field
-note: the type is defined here
-  --> $DIR/lint_uninhabited.rs:12:1
-   |
-LL | struct AlsoUninhabited{
-   | ^^^^^^^^^^^^^^^^^^^^^^
-   = help: if you meant to have a function that never returns, consider setting its return type to the never type (`!`) or a zero-variant enum
-   = note: zero-variant enums and other uninhabited types are only allowed in function returns if used directly
-note: the type is defined here
-  --> $DIR/lint_uninhabited.rs:9:1
-   |
-LL | enum Uninhabited{}
-   | ^^^^^^^^^^^^^^^^
-
 error: `extern` fn uses type `Uninhabited`, which is not FFI-safe
   --> $DIR/lint_uninhabited.rs:56:34
    |
@@ -155,5 +117,5 @@ LL | struct HalfHiddenUninhabited {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: the never type (`!`) and other uninhabited types are not allowed in function arguments and static variables
 
-error: aborting due to 9 previous errors; 1 warning emitted
+error: aborting due to 7 previous errors; 1 warning emitted
 
