Skip to content

Commit 8ca1a3d

Browse files
authored
Rollup merge of rust-lang#136193 - oli-obk:pattern-type-ffi-checks, r=chenyukang
Implement pattern type ffi checks Previously we just rejected pattern types outright in FFI, but that was never meant to be a permanent situation. We'll need them supported to use them as the building block for `NonZero` and `NonNull` after all (both of which are FFI safe). best reviewed commit by commit.
2 parents 50922da + 60ed9db commit 8ca1a3d

File tree

8 files changed

+178
-88
lines changed

8 files changed

+178
-88
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,6 @@ lint_improper_ctypes_only_phantomdata = composed only of `PhantomData`
390390
391391
lint_improper_ctypes_opaque = opaque types have no C equivalent
392392
393-
lint_improper_ctypes_pat_help = consider using the base type instead
394-
395-
lint_improper_ctypes_pat_reason = pattern types have no C equivalent
396393
lint_improper_ctypes_slice_help = consider using a raw pointer instead
397394
398395
lint_improper_ctypes_slice_reason = slices have no C equivalent

compiler/rustc_lint/src/foreign_modules.rs

Lines changed: 3 additions & 6 deletions
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/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#![feature(rustc_attrs)]
3434
#![feature(rustdoc_internals)]
3535
#![feature(trait_upcasting)]
36+
#![feature(try_blocks)]
3637
#![warn(unreachable_pub)]
3738
// tidy-alphabetical-end
3839

compiler/rustc_lint/src/types.rs

Lines changed: 91 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,37 @@ fn ty_is_known_nonnull<'tcx>(
870870
.filter_map(|variant| transparent_newtype_field(tcx, variant))
871871
.any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args), mode))
872872
}
873+
ty::Pat(base, pat) => {
874+
ty_is_known_nonnull(tcx, typing_env, *base, mode)
875+
|| Option::unwrap_or_default(
876+
try {
877+
match **pat {
878+
ty::PatternKind::Range { start, end, include_end } => {
879+
match (start, end) {
880+
(Some(start), None) => {
881+
start.try_to_value()?.try_to_bits(tcx, typing_env)? > 0
882+
}
883+
(Some(start), Some(end)) => {
884+
let start =
885+
start.try_to_value()?.try_to_bits(tcx, typing_env)?;
886+
let end =
887+
end.try_to_value()?.try_to_bits(tcx, typing_env)?;
888+
889+
if include_end {
890+
// This also works for negative numbers, as we just need
891+
// to ensure we aren't wrapping over zero.
892+
start > 0 && end >= start
893+
} else {
894+
start > 0 && end > start
895+
}
896+
}
897+
_ => false,
898+
}
899+
}
900+
}
901+
},
902+
)
903+
}
873904
_ => false,
874905
}
875906
}
@@ -900,9 +931,8 @@ fn get_nullable_type<'tcx>(
900931
};
901932
return get_nullable_type(tcx, typing_env, inner_field_ty);
902933
}
903-
ty::Int(ty) => Ty::new_int(tcx, ty),
904-
ty::Uint(ty) => Ty::new_uint(tcx, ty),
905-
ty::RawPtr(ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
934+
ty::Pat(base, ..) => return get_nullable_type(tcx, typing_env, base),
935+
ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => ty,
906936
// As these types are always non-null, the nullable equivalent of
907937
// `Option<T>` of these types are their raw pointer counterparts.
908938
ty::Ref(_region, ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
@@ -958,63 +988,69 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
958988
ckind: CItemKind,
959989
) -> Option<Ty<'tcx>> {
960990
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
961-
if let ty::Adt(ty_def, args) = ty.kind() {
962-
let field_ty = match &ty_def.variants().raw[..] {
963-
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
964-
([], [field]) | ([field], []) => field.ty(tcx, args),
965-
([field1], [field2]) => {
966-
let ty1 = field1.ty(tcx, args);
967-
let ty2 = field2.ty(tcx, args);
968-
969-
if is_niche_optimization_candidate(tcx, typing_env, ty1) {
970-
ty2
971-
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
972-
ty1
973-
} else {
974-
return None;
991+
match ty.kind() {
992+
ty::Adt(ty_def, args) => {
993+
let field_ty = match &ty_def.variants().raw[..] {
994+
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
995+
([], [field]) | ([field], []) => field.ty(tcx, args),
996+
([field1], [field2]) => {
997+
let ty1 = field1.ty(tcx, args);
998+
let ty2 = field2.ty(tcx, args);
999+
1000+
if is_niche_optimization_candidate(tcx, typing_env, ty1) {
1001+
ty2
1002+
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
1003+
ty1
1004+
} else {
1005+
return None;
1006+
}
9751007
}
976-
}
1008+
_ => return None,
1009+
},
9771010
_ => return None,
978-
},
979-
_ => return None,
980-
};
1011+
};
9811012

982-
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
983-
return None;
984-
}
1013+
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
1014+
return None;
1015+
}
9851016

986-
// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
987-
// If the computed size for the field and the enum are different, the nonnull optimization isn't
988-
// being applied (and we've got a problem somewhere).
989-
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
990-
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
991-
bug!("improper_ctypes: Option nonnull optimization not applied?");
992-
}
1017+
// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
1018+
// If the computed size for the field and the enum are different, the nonnull optimization isn't
1019+
// being applied (and we've got a problem somewhere).
1020+
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
1021+
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
1022+
bug!("improper_ctypes: Option nonnull optimization not applied?");
1023+
}
9931024

994-
// Return the nullable type this Option-like enum can be safely represented with.
995-
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
996-
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
997-
bug!("should be able to compute the layout of non-polymorphic type");
998-
}
1025+
// Return the nullable type this Option-like enum can be safely represented with.
1026+
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
1027+
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
1028+
bug!("should be able to compute the layout of non-polymorphic type");
1029+
}
9991030

1000-
let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
1001-
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
1002-
match field_ty_scalar.valid_range(&tcx) {
1003-
WrappingRange { start: 0, end }
1004-
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
1005-
{
1006-
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1007-
}
1008-
WrappingRange { start: 1, .. } => {
1009-
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1010-
}
1011-
WrappingRange { start, end } => {
1012-
unreachable!("Unhandled start and end range: ({}, {})", start, end)
1013-
}
1014-
};
1031+
let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
1032+
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
1033+
match field_ty_scalar.valid_range(&tcx) {
1034+
WrappingRange { start: 0, end }
1035+
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
1036+
{
1037+
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1038+
}
1039+
WrappingRange { start: 1, .. } => {
1040+
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
1041+
}
1042+
WrappingRange { start, end } => {
1043+
unreachable!("Unhandled start and end range: ({}, {})", start, end)
1044+
}
1045+
};
1046+
}
1047+
None
10151048
}
1049+
ty::Pat(base, pat) => match **pat {
1050+
ty::PatternKind::Range { .. } => get_nullable_type(tcx, typing_env, *base),
1051+
},
1052+
_ => None,
10161053
}
1017-
None
10181054
}
10191055

10201056
impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
@@ -1249,11 +1285,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12491285
help: Some(fluent::lint_improper_ctypes_char_help),
12501286
},
12511287

1252-
ty::Pat(..) => FfiUnsafe {
1253-
ty,
1254-
reason: fluent::lint_improper_ctypes_pat_reason,
1255-
help: Some(fluent::lint_improper_ctypes_pat_help),
1256-
},
1288+
// It's just extra invariants on the type that you need to uphold,
1289+
// but only the base type is relevant for being representable in FFI.
1290+
ty::Pat(base, ..) => self.check_type_for_ffi(acc, base),
12571291

12581292
ty::Int(ty::IntTy::I128) | ty::Uint(ty::UintTy::U128) => {
12591293
FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_128bit, help: None }

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//@ check-pass
22
//@ aux-build:external_extern_fn.rs
33
#![crate_type = "lib"]
4-
4+
#![feature(pattern_type_macro, pattern_types)]
55
mod redeclared_different_signature {
66
mod a {
77
extern "C" {
@@ -490,3 +490,33 @@ mod hidden_niche {
490490
}
491491
}
492492
}
493+
494+
mod pattern_types {
495+
mod a {
496+
use std::pat::pattern_type;
497+
#[repr(transparent)]
498+
struct NonZeroUsize(pattern_type!(usize is 1..));
499+
extern "C" {
500+
fn pt_non_zero_usize() -> pattern_type!(usize is 1..);
501+
fn pt_non_zero_usize_opt() -> Option<pattern_type!(usize is 1..)>;
502+
fn pt_non_zero_usize_opt_full_range() -> Option<pattern_type!(usize is 0..)>;
503+
//~^ WARN not FFI-safe
504+
fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
505+
fn pt_non_zero_usize_wrapper() -> NonZeroUsize;
506+
fn pt_non_zero_usize_wrapper_opt() -> Option<NonZeroUsize>;
507+
}
508+
}
509+
mod b {
510+
extern "C" {
511+
// If there's a clash in either of these cases you're either gaining an incorrect
512+
// invariant that the value is non-zero, or you're missing out on that invariant. Both
513+
// cases are warning for, from both a caller-convenience and optimisation perspective.
514+
fn pt_non_zero_usize() -> usize;
515+
fn pt_non_zero_usize_opt() -> usize;
516+
fn pt_non_null_ptr() -> *const ();
517+
//~^ WARN `pt_non_null_ptr` redeclared with a different signature
518+
fn pt_non_zero_usize_wrapper() -> usize;
519+
fn pt_non_zero_usize_wrapper_opt() -> usize;
520+
}
521+
}
522+
}

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
1717
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
1818
= note: enum has no representation hint
1919

20+
warning: `extern` block uses type `Option<(usize) is 0..=>`, which is not FFI-safe
21+
--> $DIR/clashing-extern-fn.rs:502:54
22+
|
23+
LL | fn pt_non_zero_usize_opt_full_range() -> Option<pattern_type!(usize is 0..)>;
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
25+
|
26+
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
27+
= note: enum has no representation hint
28+
2029
warning: `clash` redeclared with a different signature
2130
--> $DIR/clashing-extern-fn.rs:13:13
2231
|
@@ -258,5 +267,17 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
258267
= note: expected `unsafe extern "C" fn() -> usize`
259268
found `unsafe extern "C" fn() -> Option<UnsafeCell<NonZero<usize>>>`
260269

261-
warning: 22 warnings emitted
270+
warning: `pt_non_null_ptr` redeclared with a different signature
271+
--> $DIR/clashing-extern-fn.rs:516:13
272+
|
273+
LL | fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
274+
| ---------------------------------------------------- `pt_non_null_ptr` previously declared here
275+
...
276+
LL | fn pt_non_null_ptr() -> *const ();
277+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
278+
|
279+
= note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
280+
found `unsafe extern "C" fn() -> *const ()`
281+
282+
warning: 24 warnings emitted
262283

tests/ui/lint/lint-ctypes-enum.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ extern "C" {
9494
fn option_transparent_union(x: Option<TransparentUnion<num::NonZero<u8>>>);
9595
//~^ ERROR `extern` block uses type
9696
fn option_repr_rust(x: Option<Rust<num::NonZero<u8>>>); //~ ERROR `extern` block uses type
97+
fn option_u8(x: Option<u8>); //~ ERROR `extern` block uses type
9798

9899
fn result_ref_t(x: Result<&'static u8, ()>);
99100
fn result_fn_t(x: Result<extern "C" fn(), ()>);

0 commit comments

Comments
 (0)