lint ImproperCTypes: overhaul (take 2 of "better handling of indirections")

compiler/rustc_codegen_cranelift/example/std_example.rs

@@ -209,7 +209,7 @@ fn rust_call_abi() {
 struct I64X2([i64; 2]);
 
 #[cfg_attr(target_arch = "s390x", allow(dead_code))]
-#[allow(improper_ctypes_definitions)]
+#[allow(improper_c_fn_definitions)]
 extern "C" fn foo(_a: I64X2) {}
 
 #[cfg(target_arch = "x86_64")]

compiler/rustc_lint/messages.ftl

@@ -350,21 +350,29 @@ lint_implicit_unsafe_autorefs = implicit autoref creates a reference to the dere
     .method_def = method calls to `{$method_name}` require a reference
     .suggestion = try using a raw pointer method instead; or if this reference is intentional, make it explicit
 
-lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe
+lint_improper_ctypes = {$desc} uses type `{$ty}`, which is not FFI-safe
     .label = not FFI-safe
     .note = the type is defined here
 
 lint_improper_ctypes_array_help = consider passing a pointer to the array
 
 lint_improper_ctypes_array_reason = passing raw arrays by value is not FFI-safe
-lint_improper_ctypes_box = box cannot be represented as a single pointer
 
 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_cstr_help =
-    consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+lint_improper_ctypes_cstr_help_const =
+    consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` or `CString::as_ptr()`
+lint_improper_ctypes_cstr_help_mut =
+    consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+lint_improper_ctypes_cstr_help_owned =
+    consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+    and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+    (note that `CString::into_raw()`'s output must not be `libc::free()`'d)
+lint_improper_ctypes_cstr_help_unknown =
+    consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+    and use (depending on the use case) `CStr::as_ptr()`, `CString::into_raw()` then `CString::from_raw()`, or a dedicated buffer
 lint_improper_ctypes_cstr_reason = `CStr`/`CString` do not have a guaranteed layout
 
 lint_improper_ctypes_dyn = trait objects have no C equivalent
@@ -375,40 +383,51 @@ lint_improper_ctypes_enum_repr_help =
 lint_improper_ctypes_enum_repr_reason = enum has no representation hint
 lint_improper_ctypes_fnptr_help = consider using an `extern fn(...) -> ...` function pointer instead
 
+lint_improper_ctypes_fnptr_indirect_reason = the function pointer to `{$ty}` is FFI-unsafe due to `{$inner_ty}`
 lint_improper_ctypes_fnptr_reason = this function pointer has Rust-specific calling convention
+
 lint_improper_ctypes_non_exhaustive = this enum is non-exhaustive
 lint_improper_ctypes_non_exhaustive_variant = this enum has non-exhaustive variants
 
 lint_improper_ctypes_only_phantomdata = composed only of `PhantomData`
 
 lint_improper_ctypes_opaque = opaque types have no C equivalent
 
-lint_improper_ctypes_slice_help = consider using a raw pointer instead
-
+lint_improper_ctypes_slice_help = consider using a raw pointer to the slice's first element (and a length) instead
 lint_improper_ctypes_slice_reason = slices have no C equivalent
-lint_improper_ctypes_str_help = consider using `*const u8` and a length instead
 
+lint_improper_ctypes_str_help = consider using `*const u8` and a length instead
 lint_improper_ctypes_str_reason = string slices have no C equivalent
-lint_improper_ctypes_struct_fieldless_help = consider adding a member to this struct
 
-lint_improper_ctypes_struct_fieldless_reason = this struct has no fields
-lint_improper_ctypes_struct_layout_help = consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
+lint_improper_ctypes_struct_consider_transparent = `{$ty}` has exactly one non-zero-sized field, consider making it `#[repr(transparent)]` instead
+lint_improper_ctypes_struct_dueto = this struct/enum/union (`{$ty}`) is FFI-unsafe due to a `{$inner_ty}` field
+
+lint_improper_ctypes_struct_fieldless_help = consider adding a member to this struct
+lint_improper_ctypes_struct_fieldless_reason = `{$ty}` has no fields
 
-lint_improper_ctypes_struct_layout_reason = this struct has unspecified layout
-lint_improper_ctypes_struct_non_exhaustive = this struct is non-exhaustive
-lint_improper_ctypes_struct_zst = this struct contains only zero-sized fields
+lint_improper_ctypes_struct_layout_help = consider adding a `#[repr(C)]` (not `#[repr(C,packed)]`) or `#[repr(transparent)]` attribute to `{$ty}`
+lint_improper_ctypes_struct_layout_reason = `{$ty}` has unspecified layout
+lint_improper_ctypes_struct_non_exhaustive = `{$ty}` is non-exhaustive
+lint_improper_ctypes_struct_zst = `{$ty}` contains only zero-sized fields
 
 lint_improper_ctypes_tuple_help = consider using a struct instead
-
 lint_improper_ctypes_tuple_reason = tuples have unspecified layout
-lint_improper_ctypes_union_fieldless_help = consider adding a member to this union
 
+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_never = the never type (`!`) and other uninhabited types are not allowed in function arguments and static variables
+
+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
 lint_improper_ctypes_union_fieldless_reason = this union has no fields
-lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this union
+lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` attribute to this union
 
 lint_improper_ctypes_union_layout_reason = this union has unspecified layout
 lint_improper_ctypes_union_non_exhaustive = this union is non-exhaustive
 
+lint_improper_ctypes_unsized_box = this box for an unsized type contains metadata, which makes it incompatible with a C pointer
+lint_improper_ctypes_unsized_ptr = this pointer to an unsized type contains metadata, which makes it incompatible with a C pointer
+lint_improper_ctypes_unsized_ref = this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
+
 lint_incomplete_include =
     include macro expected single expression in source
 

compiler/rustc_lint/src/foreign_modules.rs

@@ -136,7 +136,6 @@ impl ClashingExternDeclarations {
             ty::TypingEnv::non_body_analysis(tcx, this_fi.owner_id),
             existing_decl_ty,
             this_decl_ty,
-            types::CItemKind::Declaration,
         ) {
             let orig = name_of_extern_decl(tcx, existing_did);
 
@@ -214,10 +213,9 @@ fn structurally_same_type<'tcx>(
     typing_env: ty::TypingEnv<'tcx>,
     a: Ty<'tcx>,
     b: Ty<'tcx>,
-    ckind: types::CItemKind,
 ) -> bool {
     let mut seen_types = UnordSet::default();
-    let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b, ckind);
+    let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b);
     if cfg!(debug_assertions) && result {
         // Sanity-check: must have same ABI, size and alignment.
         // `extern` blocks cannot be generic, so we'll always get a layout here.
@@ -236,7 +234,6 @@ fn structurally_same_type_impl<'tcx>(
     typing_env: ty::TypingEnv<'tcx>,
     a: Ty<'tcx>,
     b: Ty<'tcx>,
-    ckind: types::CItemKind,
 ) -> bool {
     debug!("structurally_same_type_impl(tcx, a = {:?}, b = {:?})", a, b);
 
@@ -307,33 +304,26 @@ fn structurally_same_type_impl<'tcx>(
                                 typing_env,
                                 tcx.type_of(a_did).instantiate(tcx, a_gen_args),
                                 tcx.type_of(b_did).instantiate(tcx, b_gen_args),
-                                ckind,
                             )
                         },
                     )
                 }
                 (ty::Array(a_ty, a_len), ty::Array(b_ty, b_len)) => {
                     // For arrays, we also check the length.
                     a_len == b_len
-                        && structurally_same_type_impl(
-                            seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
-                        )
+                        && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
                 }
                 (ty::Slice(a_ty), ty::Slice(b_ty)) => {
-                    structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty, ckind)
+                    structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
                 }
                 (ty::RawPtr(a_ty, a_mutbl), ty::RawPtr(b_ty, b_mutbl)) => {
                     a_mutbl == b_mutbl
-                        && structurally_same_type_impl(
-                            seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
-                        )
+                        && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
                 }
                 (ty::Ref(_a_region, a_ty, a_mut), ty::Ref(_b_region, b_ty, b_mut)) => {
                     // For structural sameness, we don't need the region to be same.
                     a_mut == b_mut
-                        && structurally_same_type_impl(
-                            seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
-                        )
+                        && structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
                 }
                 (ty::FnDef(..), ty::FnDef(..)) => {
                     let a_poly_sig = a.fn_sig(tcx);
@@ -347,15 +337,14 @@ fn structurally_same_type_impl<'tcx>(
                     (a_sig.abi, a_sig.safety, a_sig.c_variadic)
                         == (b_sig.abi, b_sig.safety, b_sig.c_variadic)
                         && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
-                            structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b, ckind)
+                            structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b)
                         })
                         && structurally_same_type_impl(
                             seen_types,
                             tcx,
                             typing_env,
                             a_sig.output(),
                             b_sig.output(),
-                            ckind,
                         )
                 }
                 (ty::Tuple(..), ty::Tuple(..)) => {
@@ -383,14 +372,14 @@ fn structurally_same_type_impl<'tcx>(
                 // An Adt and a primitive or pointer type. This can be FFI-safe if non-null
                 // enum layout optimisation is being applied.
                 (ty::Adt(..) | ty::Pat(..), _) if is_primitive_or_pointer(b) => {
-                    if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) {
+                    if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a) {
                         a_inner == b
                     } else {
                         false
                     }
                 }
                 (_, ty::Adt(..) | ty::Pat(..)) if is_primitive_or_pointer(a) => {
-                    if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) {
+                    if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b) {
                         b_inner == a
                     } else {
                         false

compiler/rustc_lint/src/lib.rs

@@ -194,8 +194,7 @@ late_lint_methods!(
             DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
             DerefIntoDynSupertrait: DerefIntoDynSupertrait,
             DropForgetUseless: DropForgetUseless,
-            ImproperCTypesDeclarations: ImproperCTypesDeclarations,
-            ImproperCTypesDefinitions: ImproperCTypesDefinitions,
+            ImproperCTypesLint: ImproperCTypesLint,
             InvalidFromUtf8: InvalidFromUtf8,
             VariantSizeDifferences: VariantSizeDifferences,
             PathStatements: PathStatements,
@@ -336,6 +335,18 @@ fn register_builtins(store: &mut LintStore) {
         REFINING_IMPL_TRAIT_INTERNAL
     );
 
+    add_lint_group!(
+        "improper_c_boundaries",
+        IMPROPER_C_CALLBACKS,
+        IMPROPER_C_FN_DEFINITIONS,
+        IMPROPER_C_VAR_DEFINITIONS,
+        IMPROPER_CTYPES
+    );
+
+    // FIXME(ctypes, migration): when should this retrocompatibility-borne
+    // lint group disappear, if at all? Should it turn into a register_renamed?
+    add_lint_group!("improper_ctypes_definitions", IMPROPER_C_CALLBACKS, IMPROPER_C_FN_DEFINITIONS);
+
     add_lint_group!("deprecated_safe", DEPRECATED_SAFE_2024);
 
     add_lint_group!(

compiler/rustc_lint/src/lints.rs

@@ -1943,29 +1943,55 @@ pub(crate) enum UnpredictableFunctionPointerComparisonsSuggestion<'a, 'tcx> {
     },
 }
 
+pub(crate) struct ImproperCTypesLayer<'a> {
+    pub ty: Ty<'a>,
+    pub inner_ty: Option<Ty<'a>>,
+    pub note: DiagMessage,
+    pub span_note: Option<Span>,
+    pub help: Option<DiagMessage>,
+}
+
+impl<'a> Subdiagnostic for ImproperCTypesLayer<'a> {
+    fn add_to_diag<G: EmissionGuarantee>(self, diag: &mut Diag<'_, G>) {
+        diag.arg("ty", self.ty);
+        if let Some(ty) = self.inner_ty {
+            diag.arg("inner_ty", ty);
+        }
+
+        if let Some(help) = self.help {
+            diag.help(diag.eagerly_translate(help));
+        }
+
+        diag.note(diag.eagerly_translate(self.note));
+        if let Some(note) = self.span_note {
+            diag.span_note(note, fluent::lint_note);
+        };
+
+        diag.remove_arg("ty");
+        if self.inner_ty.is_some() {
+            diag.remove_arg("inner_ty");
+        }
+    }
+}
+
 pub(crate) struct ImproperCTypes<'a> {
     pub ty: Ty<'a>,
     pub desc: &'a str,
     pub label: Span,
-    pub help: Option<DiagMessage>,
-    pub note: DiagMessage,
-    pub span_note: Option<Span>,
+    pub reasons: Vec<ImproperCTypesLayer<'a>>,
 }
 
 // Used because of the complexity of Option<DiagMessage>, DiagMessage, and Option<Span>
 impl<'a> LintDiagnostic<'a, ()> for ImproperCTypes<'_> {
     fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, ()>) {
         diag.primary_message(fluent::lint_improper_ctypes);
-        diag.arg("ty", self.ty);
-        diag.arg("desc", self.desc);
         diag.span_label(self.label, fluent::lint_label);
-        if let Some(help) = self.help {
-            diag.help(help);
-        }
-        diag.note(self.note);
-        if let Some(note) = self.span_note {
-            diag.span_note(note, fluent::lint_note);
+        for reason in self.reasons.into_iter() {
+            diag.subdiagnostic(reason);
         }
+        // declare the arguments at the end to avoid them being clobbered in the subdiagnostics
+        diag.arg("ty", self.ty);
+        diag.arg("desc", self.desc);
     }
 }