lint ImproperCTypes: misc. changes due to review

- relaxed the uninhabited types allowed on function return

Author: niacdoial

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

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)
         }
     }
 
@@ -769,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
@@ -780,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)
@@ -874,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
@@ -980,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>,
@@ -990,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.
@@ -1047,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?)
@@ -1107,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),
                 }
             }
 
@@ -1242,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

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,

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