Correctly handle pattern types in FFI redeclaration lints

oli-obk · oli-obk · commit dd5927db8181 · 2025-01-28T11:22:34.000Z
diff --git a/compiler/rustc_lint/src/foreign_modules.rs b/compiler/rustc_lint/src/foreign_modules.rs
@@ -241,10 +241,7 @@ fn structurally_same_type_impl<'tcx>(
             if let ty::Adt(def, args) = *ty.kind() {
                 let is_transparent = def.repr().transparent();
                 let is_non_null = types::nonnull_optimization_guaranteed(tcx, def);
-                debug!(
-                    "non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}",
-                    ty, is_transparent, is_non_null
-                );
+                debug!(?ty, is_transparent, is_non_null);
                 if is_transparent && !is_non_null {
                     debug_assert_eq!(def.variants().len(), 1);
                     let v = &def.variant(FIRST_VARIANT);
@@ -378,14 +375,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.
-                (Adt(..), _) if is_primitive_or_pointer(b) => {
+                (Adt(..) | Pat(..), _) if is_primitive_or_pointer(b) => {
                     if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) {
                         a_inner == b
                     } else {
                         false
                     }
                 }
-                (_, Adt(..)) if is_primitive_or_pointer(a) => {
+                (_, Adt(..) | Pat(..)) if is_primitive_or_pointer(a) => {
                     if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) {
                         b_inner == a
                     } else {
diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs
@@ -891,9 +891,8 @@ fn get_nullable_type<'tcx>(
             };
             return get_nullable_type(tcx, typing_env, inner_field_ty);
         }
-        ty::Int(ty) => Ty::new_int(tcx, ty),
-        ty::Uint(ty) => Ty::new_uint(tcx, ty),
-        ty::RawPtr(ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
+        ty::Pat(base, ..) => return get_nullable_type(tcx, typing_env, base),
+        ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => ty,
         // As these types are always non-null, the nullable equivalent of
         // `Option<T>` of these types are their raw pointer counterparts.
         ty::Ref(_region, ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
@@ -949,63 +948,69 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
     ckind: CItemKind,
 ) -> Option<Ty<'tcx>> {
     debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
-    if let ty::Adt(ty_def, args) = ty.kind() {
-        let field_ty = match &ty_def.variants().raw[..] {
-            [var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
-                ([], [field]) | ([field], []) => field.ty(tcx, args),
-                ([field1], [field2]) => {
-                    let ty1 = field1.ty(tcx, args);
-                    let ty2 = field2.ty(tcx, args);
-
-                    if is_niche_optimization_candidate(tcx, typing_env, ty1) {
-                        ty2
-                    } else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
-                        ty1
-                    } else {
-                        return None;
+    match ty.kind() {
+        ty::Adt(ty_def, args) => {
+            let field_ty = match &ty_def.variants().raw[..] {
+                [var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
+                    ([], [field]) | ([field], []) => field.ty(tcx, args),
+                    ([field1], [field2]) => {
+                        let ty1 = field1.ty(tcx, args);
+                        let ty2 = field2.ty(tcx, args);
+
+                        if is_niche_optimization_candidate(tcx, typing_env, ty1) {
+                            ty2
+                        } else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
+                            ty1
+                        } else {
+                            return None;
+                        }
                     }
-                }
+                    _ => return None,
+                },
                 _ => return None,
-            },
-            _ => return None,
-        };
+            };
 
-        if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
-            return None;
-        }
+            if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
+                return None;
+            }
 
-        // At this point, the field's type is known to be nonnull and the parent enum is Option-like.
-        // If the computed size for the field and the enum are different, the nonnull optimization isn't
-        // being applied (and we've got a problem somewhere).
-        let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
-        if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
-            bug!("improper_ctypes: Option nonnull optimization not applied?");
-        }
+            // At this point, the field's type is known to be nonnull and the parent enum is Option-like.
+            // If the computed size for the field and the enum are different, the nonnull optimization isn't
+            // being applied (and we've got a problem somewhere).
+            let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
+            if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
+                bug!("improper_ctypes: Option nonnull optimization not applied?");
+            }
 
-        // Return the nullable type this Option-like enum can be safely represented with.
-        let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
-        if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
-            bug!("should be able to compute the layout of non-polymorphic type");
-        }
+            // Return the nullable type this Option-like enum can be safely represented with.
+            let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
+            if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
+                bug!("should be able to compute the layout of non-polymorphic type");
+            }
 
-        let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
-        if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
-            match field_ty_scalar.valid_range(&tcx) {
-                WrappingRange { start: 0, end }
-                    if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
-                {
-                    return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
-                }
-                WrappingRange { start: 1, .. } => {
-                    return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
-                }
-                WrappingRange { start, end } => {
-                    unreachable!("Unhandled start and end range: ({}, {})", start, end)
-                }
-            };
+            let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
+            if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
+                match field_ty_scalar.valid_range(&tcx) {
+                    WrappingRange { start: 0, end }
+                        if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
+                    {
+                        return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
+                    }
+                    WrappingRange { start: 1, .. } => {
+                        return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
+                    }
+                    WrappingRange { start, end } => {
+                        unreachable!("Unhandled start and end range: ({}, {})", start, end)
+                    }
+                };
+            }
+            None
         }
+        ty::Pat(base, pat) => match **pat {
+            ty::PatternKind::Range { .. } => get_nullable_type(tcx, typing_env, *base),
+        },
+        _ => None,
     }
-    None
 }
 
 impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
diff --git a/tests/ui/lint/clashing-extern-fn.rs b/tests/ui/lint/clashing-extern-fn.rs
@@ -512,13 +512,11 @@ mod pattern_types {
             // invariant that the value is non-zero, or you're missing out on that invariant. Both
             // cases are warning for, from both a caller-convenience and optimisation perspective.
             fn pt_non_zero_usize() -> usize;
-            //~^ WARN `pt_non_zero_usize` redeclared with a different signature
             fn pt_non_zero_usize_opt() -> usize;
             //~^ WARN `pt_non_zero_usize_opt` redeclared with a different signature
             fn pt_non_null_ptr() -> *const ();
             //~^ WARN `pt_non_null_ptr` redeclared with a different signature
             fn pt_non_zero_usize_wrapper() -> usize;
-            //~^ WARN `pt_non_zero_usize_wrapper` redeclared with a different signature
             fn pt_non_zero_usize_wrapper_opt() -> usize;
             //~^ WARN `pt_non_zero_usize_wrapper_opt` redeclared with a different signature
         }
diff --git a/tests/ui/lint/clashing-extern-fn.stderr b/tests/ui/lint/clashing-extern-fn.stderr
@@ -276,20 +276,8 @@ LL |             fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
    = note: expected `unsafe extern "C" fn() -> usize`
               found `unsafe extern "C" fn() -> Option<UnsafeCell<NonZero<usize>>>`
 
-warning: `pt_non_zero_usize` redeclared with a different signature
-  --> $DIR/clashing-extern-fn.rs:514:13
-   |
-LL |             fn pt_non_zero_usize() -> pattern_type!(usize is 1..);
-   |             ------------------------------------------------------ `pt_non_zero_usize` previously declared here
-...
-LL |             fn pt_non_zero_usize() -> usize;
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
-   |
-   = note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
-              found `unsafe extern "C" fn() -> usize`
-
 warning: `pt_non_zero_usize_opt` redeclared with a different signature
-  --> $DIR/clashing-extern-fn.rs:516:13
+  --> $DIR/clashing-extern-fn.rs:515:13
    |
 LL |             fn pt_non_zero_usize_opt() -> Option<pattern_type!(usize is 1..)>;
    |             ------------------------------------------------------------------ `pt_non_zero_usize_opt` previously declared here
@@ -301,7 +289,7 @@ LL |             fn pt_non_zero_usize_opt() -> usize;
               found `unsafe extern "C" fn() -> usize`
 
 warning: `pt_non_null_ptr` redeclared with a different signature
-  --> $DIR/clashing-extern-fn.rs:518:13
+  --> $DIR/clashing-extern-fn.rs:517:13
    |
 LL |             fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
    |             ---------------------------------------------------- `pt_non_null_ptr` previously declared here
@@ -312,20 +300,8 @@ LL |             fn pt_non_null_ptr() -> *const ();
    = note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
               found `unsafe extern "C" fn() -> *const ()`
 
-warning: `pt_non_zero_usize_wrapper` redeclared with a different signature
-  --> $DIR/clashing-extern-fn.rs:520:13
-   |
-LL |             fn pt_non_zero_usize_wrapper() -> NonZeroUsize;
-   |             ----------------------------------------------- `pt_non_zero_usize_wrapper` previously declared here
-...
-LL |             fn pt_non_zero_usize_wrapper() -> usize;
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
-   |
-   = note: expected `unsafe extern "C" fn() -> NonZeroUsize`
-              found `unsafe extern "C" fn() -> usize`
-
 warning: `pt_non_zero_usize_wrapper_opt` redeclared with a different signature
-  --> $DIR/clashing-extern-fn.rs:522:13
+  --> $DIR/clashing-extern-fn.rs:520:13
    |
 LL |             fn pt_non_zero_usize_wrapper_opt() -> Option<NonZeroUsize>;
    |             ----------------------------------------------------------- `pt_non_zero_usize_wrapper_opt` previously declared here
@@ -336,5 +312,5 @@ LL |             fn pt_non_zero_usize_wrapper_opt() -> usize;
    = note: expected `unsafe extern "C" fn() -> Option<NonZeroUsize>`
               found `unsafe extern "C" fn() -> usize`
 
-warning: 29 warnings emitted
+warning: 27 warnings emitted
 
