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

compiler/rustc_lint/messages.ftl

@@ -336,21 +336,22 @@ 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, converting to/from `{$ty}` as needed
+lint_improper_ctypes_cstr_help_mut =
+    consider passing a `*mut std::ffi::c_char` instead, converting to/from `{$ty}` as needed
 lint_improper_ctypes_cstr_reason = `CStr`/`CString` do not have a guaranteed layout
 
 lint_improper_ctypes_dyn = trait objects have no C equivalent
@@ -362,39 +363,49 @@ 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_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_int_to_ptr_transmutes = transmuting an integer to a pointer creates a pointer without provenance
     .note = this is dangerous because dereferencing the resulting pointer is undefined behavior
     .note_exposed_provenance = exposed provenance semantics can be used to create a pointer based on some previously exposed provenance

compiler/rustc_lint/src/lib.rs

@@ -335,6 +335,8 @@ fn register_builtins(store: &mut LintStore) {
         REFINING_IMPL_TRAIT_INTERNAL
     );
 
+    add_lint_group!("improper_c_boundaries", IMPROPER_CTYPES_DEFINITIONS, IMPROPER_CTYPES);
+
     add_lint_group!("deprecated_safe", DEPRECATED_SAFE_2024);
 
     add_lint_group!(

compiler/rustc_lint/src/lints.rs

@@ -1950,29 +1950,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);
     }
 }
 

compiler/rustc_lint/src/types.rs

@@ -1,17 +1,19 @@
 use std::iter;
 
-use rustc_abi::{BackendRepr, TagEncoding, Variants, WrappingRange};
+use rustc_abi::{BackendRepr, Size, TagEncoding, Variants, WrappingRange};
 use rustc_hir::{Expr, ExprKind, HirId, LangItem};
 use rustc_middle::bug;
 use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton};
-use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
+use rustc_middle::ty::{self, Const, ScalarInt, Ty, TyCtxt, TypeVisitableExt};
 use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass};
 use rustc_span::{Span, Symbol, sym};
 use tracing::debug;
 use {rustc_ast as ast, rustc_hir as hir};
 
 mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations
-pub(crate) use improper_ctypes::ImproperCTypesLint;
+pub(crate) use improper_ctypes::{
+    IMPROPER_CTYPES, IMPROPER_CTYPES_DEFINITIONS, ImproperCTypesLint,
+};
 
 use crate::lints::{
     AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
@@ -705,6 +707,26 @@ pub(crate) fn transparent_newtype_field<'a, 'tcx>(
     })
 }
 
+/// for a given ADT variant, list which fields are non-1ZST
+/// (`repr(transparent)` guarantees that there is at most one)
+pub(crate) fn map_non_1zst_fields<'a, 'tcx>(
+    tcx: TyCtxt<'tcx>,
+    variant: &'a ty::VariantDef,
+) -> Vec<bool> {
+    let typing_env = ty::TypingEnv::non_body_analysis(tcx, variant.def_id);
+    variant
+        .fields
+        .iter()
+        .map(|field| {
+            let field_ty = tcx.type_of(field.did).instantiate_identity();
+            let is_1zst = tcx
+                .layout_of(typing_env.as_query_input(field_ty))
+                .is_ok_and(|layout| layout.is_1zst());
+            !is_1zst
+        })
+        .collect()
+}
+
 /// Is type known to be non-null?
 fn ty_is_known_nonnull<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -842,7 +864,7 @@ fn is_niche_optimization_candidate<'tcx>(
 /// Check if this enum can be safely exported based on the "nullable pointer optimization". If it
 /// can, return the type that `ty` can be safely converted to, otherwise return `None`.
 /// Currently restricted to function pointers, boxes, references, `core::num::NonZero`,
-/// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes.
+/// `core::ptr::NonNull`, `#[repr(transparent)]` newtypes, and int-range pattern types.
 pub(crate) fn repr_nullable_ptr<'tcx>(
     tcx: TyCtxt<'tcx>,
     typing_env: ty::TypingEnv<'tcx>,
@@ -871,6 +893,14 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
                 _ => return None,
             };
 
+            if let ty::Pat(base, pat) = field_ty.kind() {
+                if pattern_has_disallowed_values(*pat) || matches!(base.kind(), ty::Char) {
+                    return get_nullable_type_from_pat(tcx, typing_env, *base, *pat);
+                } else {
+                    return None;
+                }
+            }
+
             if !ty_is_known_nonnull(tcx, typing_env, field_ty) {
                 return None;
             }
@@ -912,6 +942,151 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
     }
 }
 
+/// Returns whether a pattern type actually has disallowed values.
+pub(crate) fn pattern_has_disallowed_values<'tcx>(pat: ty::Pattern<'tcx>) -> bool {
+    // note the logic in this function assumes that signed ints use one's complement representation,
+    // which I believe is a requirement for rust
+
+    /// Find numeric metadata on a pair of range bounds.
+    /// If None, assume that there are no bounds specified
+    /// and that this is a usize. in other words, all values are allowed.
+    fn unwrap_start_end<'tcx>(
+        start: Const<'tcx>,
+        end: Const<'tcx>,
+    ) -> (bool, Size, ScalarInt, ScalarInt) {
+        let usable_bound = match (start.try_to_value(), end.try_to_value()) {
+            (Some(ty), _) | (_, Some(ty)) => ty,
+            (None, None) => bug!(
+                "pattern range should have at least one defined value: {:?} - {:?}",
+                start,
+                end,
+            ),
+        };
+        let usable_size = usable_bound.valtree.unwrap_leaf().size();
+        let is_signed = match usable_bound.ty.kind() {
+            ty::Int(_) => true,
+            ty::Uint(_) | ty::Char => false,
+            kind @ _ => bug!("unexpected non-scalar base for pattern bounds: {:?}", kind),
+        };
+
+        let end = match end.try_to_value() {
+            Some(end) => end.valtree.unwrap_leaf(),
+            None => {
+                let max_val = if is_signed {
+                    usable_size.signed_int_max() as u128
+                } else {
+                    usable_size.unsigned_int_max()
+                };
+                ScalarInt::try_from_uint(max_val, usable_size).unwrap()
+            }
+        };
+        let start = match start.try_to_value() {
+            Some(start) => start.valtree.unwrap_leaf(),
+            None => {
+                let min_val = if is_signed {
+                    (usable_size.signed_int_min() as u128) & usable_size.unsigned_int_max()
+                } else {
+                    0_u128
+                };
+                ScalarInt::try_from_uint(min_val, usable_size).unwrap()
+            }
+        };
+        (is_signed, usable_size, start, end)
+    }
+
+    match *pat {
+        ty::PatternKind::NotNull => true,
+        ty::PatternKind::Range { start, end } => {
+            let (is_signed, scalar_size, start, end) = unwrap_start_end(start, end);
+            let (scalar_min, scalar_max) = if is_signed {
+                (
+                    (scalar_size.signed_int_min() as u128) & scalar_size.unsigned_int_max(),
+                    scalar_size.signed_int_max() as u128,
+                )
+            } else {
+                (0, scalar_size.unsigned_int_max())
+            };
+
+            (start.to_bits(scalar_size), end.to_bits(scalar_size)) != (scalar_min, scalar_max)
+        }
+        ty::PatternKind::Or(patterns) => {
+            // first, get a simplified an sorted view of the ranges
+            let (is_signed, scalar_size, mut ranges) = {
+                let (is_signed, size, start, end) = match &*patterns[0] {
+                    ty::PatternKind::Range { start, end } => unwrap_start_end(*start, *end),
+                    ty::PatternKind::Or(_) => bug!("recursive \"or\" patterns?"),
+                    ty::PatternKind::NotNull => bug!("nonnull pattern in \"or\" pattern?"),
+                };
+                (is_signed, size, vec![(start, end)])
+            };
+            let scalar_max = if is_signed {
+                scalar_size.signed_int_max() as u128
+            } else {
+                scalar_size.unsigned_int_max()
+            };
+            ranges.reserve(patterns.len() - 1);
+            for pat in patterns.iter().skip(1) {
+                match *pat {
+                    ty::PatternKind::Range { start, end } => {
+                        let (is_this_signed, this_scalar_size, start, end) =
+                            unwrap_start_end(start, end);
+                        assert_eq!(is_signed, is_this_signed);
+                        assert_eq!(scalar_size, this_scalar_size);
+                        ranges.push((start, end))
+                    }
+                    ty::PatternKind::Or(_) => bug!("recursive \"or\" patterns?"),
+                    ty::PatternKind::NotNull => bug!("nonnull pattern in \"or\" pattern?"),
+                }
+            }
+            ranges.sort_by_key(|(start, _end)| {
+                let is_positive =
+                    if is_signed { start.to_bits(scalar_size) <= scalar_max } else { true };
+                (is_positive, start.to_bits(scalar_size))
+            });
+
+            // then, range per range, look at the sizes of the gaps left in between
+            // (`prev_tail` is the highest value currently accounted for by the ranges,
+            // unless the first range has not been dealt with yet)
+            let mut prev_tail = scalar_max;
+
+            for (range_i, (start, end)) in ranges.into_iter().enumerate() {
+                let (start, end) = (start.to_bits(scalar_size), end.to_bits(scalar_size));
+
+                // if the start of the current range is lower
+                // than the current-highest-range-end, ...
+                let current_range_overlap =
+                    if is_signed && prev_tail > scalar_max && start <= scalar_max {
+                        false
+                    } else if start <= u128::overflowing_add(prev_tail, 1).0 {
+                        range_i > 0 // no overlap possible when dealing with the first range
+                    } else {
+                        false
+                    };
+                if current_range_overlap {
+                    // update the current-highest-range-end, if the current range has a higher end
+                    if is_signed {
+                        if prev_tail > scalar_max && end <= scalar_max {
+                            prev_tail = end;
+                        } else if prev_tail <= scalar_max && end > scalar_max {
+                            // nothing to do here
+                        } else {
+                            // prev_tail and end have the same sign
+                            prev_tail = u128::max(prev_tail, end)
+                        }
+                    } else {
+                        // prev_tail and end have the same sign
+                        prev_tail = u128::max(prev_tail, end)
+                    }
+                } else {
+                    // no range overlap: there are disallowed values
+                    return true;
+                }
+            }
+            prev_tail != scalar_max
+        }
+    }
+}
+
 fn get_nullable_type_from_pat<'tcx>(
     tcx: TyCtxt<'tcx>,
     typing_env: ty::TypingEnv<'tcx>,