Skip to content

Commit 5bae8ca

Browse files
committed
Correctly handle pattern types in FFI redeclaration lints
1 parent 473352d commit 5bae8ca

File tree

4 files changed

+64
-88
lines changed

4 files changed

+64
-88
lines changed

compiler/rustc_lint/src/foreign_modules.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,7 @@ fn structurally_same_type_impl<'tcx>(
241241
if let ty::Adt(def, args) = *ty.kind() {
242242
let is_transparent = def.repr().transparent();
243243
let is_non_null = types::nonnull_optimization_guaranteed(tcx, def);
244-
debug!(
245-
"non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}",
246-
ty, is_transparent, is_non_null
247-
);
244+
debug!(?ty, is_transparent, is_non_null);
248245
if is_transparent && !is_non_null {
249246
debug_assert_eq!(def.variants().len(), 1);
250247
let v = &def.variant(FIRST_VARIANT);
@@ -378,14 +375,14 @@ fn structurally_same_type_impl<'tcx>(
378375

379376
// An Adt and a primitive or pointer type. This can be FFI-safe if non-null
380377
// enum layout optimisation is being applied.
381-
(Adt(..), _) if is_primitive_or_pointer(b) => {
378+
(Adt(..) | Pat(..), _) if is_primitive_or_pointer(b) => {
382379
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) {
383380
a_inner == b
384381
} else {
385382
false
386383
}
387384
}
388-
(_, Adt(..)) if is_primitive_or_pointer(a) => {
385+
(_, Adt(..) | Pat(..)) if is_primitive_or_pointer(a) => {
389386
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) {
390387
b_inner == a
391388
} else {

compiler/rustc_lint/src/types.rs

+57-52
Original file line numberDiff line numberDiff line change
@@ -907,9 +907,8 @@ fn get_nullable_type<'tcx>(
907907
};
908908
return get_nullable_type(tcx, typing_env, inner_field_ty);
909909
}
910-
ty::Int(ty) => Ty::new_int(tcx, ty),
911-
ty::Uint(ty) => Ty::new_uint(tcx, ty),
912-
ty::RawPtr(ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
910+
ty::Pat(base, ..) => return get_nullable_type(tcx, typing_env, base),
911+
ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => ty,
913912
// As these types are always non-null, the nullable equivalent of
914913
// `Option<T>` of these types are their raw pointer counterparts.
915914
ty::Ref(_region, ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
@@ -965,63 +964,69 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
965964
ckind: CItemKind,
966965
) -> Option<Ty<'tcx>> {
967966
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
968-
if let ty::Adt(ty_def, args) = ty.kind() {
969-
let field_ty = match &ty_def.variants().raw[..] {
970-
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
971-
([], [field]) | ([field], []) => field.ty(tcx, args),
972-
([field1], [field2]) => {
973-
let ty1 = field1.ty(tcx, args);
974-
let ty2 = field2.ty(tcx, args);
975-
976-
if is_niche_optimization_candidate(tcx, typing_env, ty1) {
977-
ty2
978-
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
979-
ty1
980-
} else {
981-
return None;
967+
match ty.kind() {
968+
ty::Adt(ty_def, args) => {
969+
let field_ty = match &ty_def.variants().raw[..] {
970+
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
971+
([], [field]) | ([field], []) => field.ty(tcx, args),
972+
([field1], [field2]) => {
973+
let ty1 = field1.ty(tcx, args);
974+
let ty2 = field2.ty(tcx, args);
975+
976+
if is_niche_optimization_candidate(tcx, typing_env, ty1) {
977+
ty2
978+
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
979+
ty1
980+
} else {
981+
return None;
982+
}
982983
}
983-
}
984+
_ => return None,
985+
},
984986
_ => return None,
985-
},
986-
_ => return None,
987-
};
987+
};
988988

989-
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
990-
return None;
991-
}
989+
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
990+
return None;
991+
}
992992

993-
// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
994-
// If the computed size for the field and the enum are different, the nonnull optimization isn't
995-
// being applied (and we've got a problem somewhere).
996-
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
997-
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
998-
bug!("improper_ctypes: Option nonnull optimization not applied?");
999-
}
993+
// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
994+
// If the computed size for the field and the enum are different, the nonnull optimization isn't
995+
// being applied (and we've got a problem somewhere).
996+
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
997+
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
998+
bug!("improper_ctypes: Option nonnull optimization not applied?");
999+
}
10001000

1001-
// Return the nullable type this Option-like enum can be safely represented with.
1002-
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
1003-
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
1004-
bug!("should be able to compute the layout of non-polymorphic type");
1005-
}
1001+
// Return the nullable type this Option-like enum can be safely represented with.
1002+
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
1003+
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
1004+
bug!("should be able to compute the layout of non-polymorphic type");
1005+
}
10061006

1007-
let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
1008-
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
1009-
match field_ty_scalar.valid_range(&tcx) {
1010-
WrappingRange { start: 0, end }
1011-
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
1012-
{
1013-
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1014-
}
1015-
WrappingRange { start: 1, .. } => {
1016-
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1017-
}
1018-
WrappingRange { start, end } => {
1019-
unreachable!("Unhandled start and end range: ({}, {})", start, end)
1020-
}
1021-
};
1007+
let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
1008+
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
1009+
match field_ty_scalar.valid_range(&tcx) {
1010+
WrappingRange { start: 0, end }
1011+
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
1012+
{
1013+
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1014+
}
1015+
WrappingRange { start: 1, .. } => {
1016+
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1017+
}
1018+
WrappingRange { start, end } => {
1019+
unreachable!("Unhandled start and end range: ({}, {})", start, end)
1020+
}
1021+
};
1022+
}
1023+
None
10221024
}
1025+
ty::Pat(base, pat) => match **pat {
1026+
ty::PatternKind::Range { .. } => get_nullable_type(tcx, typing_env, *base),
1027+
},
1028+
_ => None,
10231029
}
1024-
None
10251030
}
10261031

10271032
impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

tests/ui/lint/clashing-extern-fn.rs

-2
Original file line numberDiff line numberDiff line change
@@ -514,13 +514,11 @@ mod pattern_types {
514514
// invariant that the value is non-zero, or you're missing out on that invariant. Both
515515
// cases are warning for, from both a caller-convenience and optimisation perspective.
516516
fn pt_non_zero_usize() -> usize;
517-
//~^ WARN `pt_non_zero_usize` redeclared with a different signature
518517
fn pt_non_zero_usize_opt() -> usize;
519518
//~^ WARN `pt_non_zero_usize_opt` redeclared with a different signature
520519
fn pt_non_null_ptr() -> *const ();
521520
//~^ WARN `pt_non_null_ptr` redeclared with a different signature
522521
fn pt_non_zero_usize_wrapper() -> usize;
523-
//~^ WARN `pt_non_zero_usize_wrapper` redeclared with a different signature
524522
fn pt_non_zero_usize_wrapper_opt() -> usize;
525523
//~^ WARN `pt_non_zero_usize_wrapper_opt` redeclared with a different signature
526524
}

tests/ui/lint/clashing-extern-fn.stderr

+4-28
Original file line numberDiff line numberDiff line change
@@ -285,20 +285,8 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
285285
= note: expected `unsafe extern "C" fn() -> usize`
286286
found `unsafe extern "C" fn() -> Option<UnsafeCell<NonZero<usize>>>`
287287

288-
warning: `pt_non_zero_usize` redeclared with a different signature
289-
--> $DIR/clashing-extern-fn.rs:516:13
290-
|
291-
LL | fn pt_non_zero_usize() -> pattern_type!(usize is 1..);
292-
| ------------------------------------------------------ `pt_non_zero_usize` previously declared here
293-
...
294-
LL | fn pt_non_zero_usize() -> usize;
295-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
296-
|
297-
= note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
298-
found `unsafe extern "C" fn() -> usize`
299-
300288
warning: `pt_non_zero_usize_opt` redeclared with a different signature
301-
--> $DIR/clashing-extern-fn.rs:518:13
289+
--> $DIR/clashing-extern-fn.rs:517:13
302290
|
303291
LL | fn pt_non_zero_usize_opt() -> Option<pattern_type!(usize is 1..)>;
304292
| ------------------------------------------------------------------ `pt_non_zero_usize_opt` previously declared here
@@ -310,7 +298,7 @@ LL | fn pt_non_zero_usize_opt() -> usize;
310298
found `unsafe extern "C" fn() -> usize`
311299

312300
warning: `pt_non_null_ptr` redeclared with a different signature
313-
--> $DIR/clashing-extern-fn.rs:520:13
301+
--> $DIR/clashing-extern-fn.rs:519:13
314302
|
315303
LL | fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
316304
| ---------------------------------------------------- `pt_non_null_ptr` previously declared here
@@ -321,20 +309,8 @@ LL | fn pt_non_null_ptr() -> *const ();
321309
= note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
322310
found `unsafe extern "C" fn() -> *const ()`
323311

324-
warning: `pt_non_zero_usize_wrapper` redeclared with a different signature
325-
--> $DIR/clashing-extern-fn.rs:522:13
326-
|
327-
LL | fn pt_non_zero_usize_wrapper() -> NonZeroUsize;
328-
| ----------------------------------------------- `pt_non_zero_usize_wrapper` previously declared here
329-
...
330-
LL | fn pt_non_zero_usize_wrapper() -> usize;
331-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
332-
|
333-
= note: expected `unsafe extern "C" fn() -> NonZeroUsize`
334-
found `unsafe extern "C" fn() -> usize`
335-
336312
warning: `pt_non_zero_usize_wrapper_opt` redeclared with a different signature
337-
--> $DIR/clashing-extern-fn.rs:524:13
313+
--> $DIR/clashing-extern-fn.rs:522:13
338314
|
339315
LL | fn pt_non_zero_usize_wrapper_opt() -> Option<NonZeroUsize>;
340316
| ----------------------------------------------------------- `pt_non_zero_usize_wrapper_opt` previously declared here
@@ -345,5 +321,5 @@ LL | fn pt_non_zero_usize_wrapper_opt() -> usize;
345321
= note: expected `unsafe extern "C" fn() -> Option<NonZeroUsize>`
346322
found `unsafe extern "C" fn() -> usize`
347323

348-
warning: 30 warnings emitted
324+
warning: 28 warnings emitted
349325

0 commit comments

Comments
 (0)