Skip to content

Commit 63d4d8f

Browse files
niacdoialniacdoial
committed
ImproperCTypes: more pre-emptive cleanup
Mainly, we realise that the non-null assumption on a Box<_> argument does not depend on what side of the FFI boundary the function is on. And anyway, this is not the way to deal with this assumption being maybe violated.
1 parent 3e75926 commit 63d4d8f

File tree

5 files changed

+31
-56
lines changed

5 files changed

+31
-56
lines changed

‎compiler/rustc_lint/src/foreign_modules.rs‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ impl ClashingExternDeclarations {
136136
ty::TypingEnv::non_body_analysis(tcx, this_fi.owner_id),
137137
existing_decl_ty,
138138
this_decl_ty,
139-
types::CItemKind::Declaration,
140139
) {
141140
let orig = name_of_extern_decl(tcx, existing_did);
142141

@@ -214,10 +213,9 @@ fn structurally_same_type<'tcx>(
214213
typing_env: ty::TypingEnv<'tcx>,
215214
a: Ty<'tcx>,
216215
b: Ty<'tcx>,
217-
ckind: types::CItemKind,
218216
) -> bool {
219217
let mut seen_types = UnordSet::default();
220-
let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b, ckind);
218+
let result = structurally_same_type_impl(&mut seen_types, tcx, typing_env, a, b);
221219
if cfg!(debug_assertions) && result {
222220
// Sanity-check: must have same ABI, size and alignment.
223221
// `extern` blocks cannot be generic, so we'll always get a layout here.
@@ -236,7 +234,6 @@ fn structurally_same_type_impl<'tcx>(
236234
typing_env: ty::TypingEnv<'tcx>,
237235
a: Ty<'tcx>,
238236
b: Ty<'tcx>,
239-
ckind: types::CItemKind,
240237
) -> bool {
241238
debug!("structurally_same_type_impl(tcx, a = {:?}, b = {:?})", a, b);
242239

@@ -307,33 +304,26 @@ fn structurally_same_type_impl<'tcx>(
307304
typing_env,
308305
tcx.type_of(a_did).instantiate(tcx, a_gen_args),
309306
tcx.type_of(b_did).instantiate(tcx, b_gen_args),
310-
ckind,
311307
)
312308
},
313309
)
314310
}
315311
(ty::Array(a_ty, a_len), ty::Array(b_ty, b_len)) => {
316312
// For arrays, we also check the length.
317313
a_len == b_len
318-
&& structurally_same_type_impl(
319-
seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
320-
)
314+
&& structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
321315
}
322316
(ty::Slice(a_ty), ty::Slice(b_ty)) => {
323-
structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty, ckind)
317+
structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
324318
}
325319
(ty::RawPtr(a_ty, a_mutbl), ty::RawPtr(b_ty, b_mutbl)) => {
326320
a_mutbl == b_mutbl
327-
&& structurally_same_type_impl(
328-
seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
329-
)
321+
&& structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
330322
}
331323
(ty::Ref(_a_region, a_ty, a_mut), ty::Ref(_b_region, b_ty, b_mut)) => {
332324
// For structural sameness, we don't need the region to be same.
333325
a_mut == b_mut
334-
&& structurally_same_type_impl(
335-
seen_types, tcx, typing_env, *a_ty, *b_ty, ckind,
336-
)
326+
&& structurally_same_type_impl(seen_types, tcx, typing_env, *a_ty, *b_ty)
337327
}
338328
(ty::FnDef(..), ty::FnDef(..)) => {
339329
let a_poly_sig = a.fn_sig(tcx);
@@ -347,15 +337,14 @@ fn structurally_same_type_impl<'tcx>(
347337
(a_sig.abi, a_sig.safety, a_sig.c_variadic)
348338
== (b_sig.abi, b_sig.safety, b_sig.c_variadic)
349339
&& a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| {
350-
structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b, ckind)
340+
structurally_same_type_impl(seen_types, tcx, typing_env, *a, *b)
351341
})
352342
&& structurally_same_type_impl(
353343
seen_types,
354344
tcx,
355345
typing_env,
356346
a_sig.output(),
357347
b_sig.output(),
358-
ckind,
359348
)
360349
}
361350
(ty::Tuple(..), ty::Tuple(..)) => {
@@ -383,14 +372,14 @@ fn structurally_same_type_impl<'tcx>(
383372
// An Adt and a primitive or pointer type. This can be FFI-safe if non-null
384373
// enum layout optimisation is being applied.
385374
(ty::Adt(..) | ty::Pat(..), _) if is_primitive_or_pointer(b) => {
386-
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) {
375+
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a) {
387376
a_inner == b
388377
} else {
389378
false
390379
}
391380
}
392381
(_, ty::Adt(..) | ty::Pat(..)) if is_primitive_or_pointer(a) => {
393-
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) {
382+
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b) {
394383
b_inner == a
395384
} else {
396385
false

‎compiler/rustc_lint/src/types.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ use tracing::debug;
1111
use {rustc_ast as ast, rustc_hir as hir};
1212

1313
mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations
14-
pub(crate) use improper_ctypes::{
15-
CItemKind, ImproperCTypesDeclarations, ImproperCTypesDefinitions,
16-
};
14+
pub(crate) use improper_ctypes::{ImproperCTypesDeclarations, ImproperCTypesDefinitions};
1715

1816
use crate::lints::{
1917
AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
@@ -712,14 +710,13 @@ fn ty_is_known_nonnull<'tcx>(
712710
tcx: TyCtxt<'tcx>,
713711
typing_env: ty::TypingEnv<'tcx>,
714712
ty: Ty<'tcx>,
715-
mode: CItemKind,
716713
) -> bool {
717714
let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty);
718715

719716
match ty.kind() {
720717
ty::FnPtr(..) => true,
721718
ty::Ref(..) => true,
722-
ty::Adt(def, _) if def.is_box() && matches!(mode, CItemKind::Definition) => true,
719+
ty::Adt(def, _) if def.is_box() => true,
723720
ty::Adt(def, args) if def.repr().transparent() && !def.is_union() => {
724721
let marked_non_null = nonnull_optimization_guaranteed(tcx, *def);
725722

@@ -735,10 +732,10 @@ fn ty_is_known_nonnull<'tcx>(
735732
def.variants()
736733
.iter()
737734
.filter_map(|variant| transparent_newtype_field(tcx, variant))
738-
.any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args), mode))
735+
.any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args)))
739736
}
740737
ty::Pat(base, pat) => {
741-
ty_is_known_nonnull(tcx, typing_env, *base, mode)
738+
ty_is_known_nonnull(tcx, typing_env, *base)
742739
|| pat_ty_is_known_nonnull(tcx, typing_env, *pat)
743740
}
744741
_ => false,
@@ -849,7 +846,6 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
849846
tcx: TyCtxt<'tcx>,
850847
typing_env: ty::TypingEnv<'tcx>,
851848
ty: Ty<'tcx>,
852-
ckind: CItemKind,
853849
) -> Option<Ty<'tcx>> {
854850
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
855851
match ty.kind() {
@@ -874,7 +870,7 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
874870
_ => return None,
875871
};
876872

877-
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
873+
if !ty_is_known_nonnull(tcx, typing_env, field_ty) {
878874
return None;
879875
}
880876

‎compiler/rustc_lint/src/types/improper_ctypes.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool {
173173
}
174174

175175
#[derive(Clone, Copy)]
176-
pub(crate) enum CItemKind {
176+
enum CItemKind {
177177
Declaration,
178178
Definition,
179179
}
@@ -385,7 +385,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
385385
{
386386
// Special-case types like `Option<extern fn()>` and `Result<extern fn(), ()>`
387387
if let Some(ty) =
388-
repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty, self.mode)
388+
repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty)
389389
{
390390
return self.check_type_for_ffi(acc, ty);
391391
}

‎tests/ui/lint/improper_ctypes/lint-ctypes.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ extern "C" {
5252
pub fn str_type(p: &str); //~ ERROR: uses type `str`
5353
pub fn box_type(p: Box<u32>); //~ ERROR uses type `Box<u32>`
5454
pub fn opt_box_type(p: Option<Box<u32>>);
55-
//~^ ERROR uses type `Option<Box<u32>>`
5655
pub fn char_type(p: char); //~ ERROR uses type `char`
5756
pub fn trait_type(p: &dyn Bar); //~ ERROR uses type `dyn Bar`
5857
pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)`

‎tests/ui/lint/improper_ctypes/lint-ctypes.stderr‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,8 @@ LL | pub fn box_type(p: Box<u32>);
6767
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
6868
= note: this struct has unspecified layout
6969

70-
error: `extern` block uses type `Option<Box<u32>>`, which is not FFI-safe
71-
--> $DIR/lint-ctypes.rs:54:28
72-
|
73-
LL | pub fn opt_box_type(p: Option<Box<u32>>);
74-
| ^^^^^^^^^^^^^^^^ not FFI-safe
75-
|
76-
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
77-
= note: enum has no representation hint
78-
7970
error: `extern` block uses type `char`, which is not FFI-safe
80-
--> $DIR/lint-ctypes.rs:56:25
71+
--> $DIR/lint-ctypes.rs:55:25
8172
|
8273
LL | pub fn char_type(p: char);
8374
| ^^^^ not FFI-safe
@@ -86,15 +77,15 @@ LL | pub fn char_type(p: char);
8677
= note: the `char` type has no C equivalent
8778

8879
error: `extern` block uses type `dyn Bar`, which is not FFI-safe
89-
--> $DIR/lint-ctypes.rs:57:26
80+
--> $DIR/lint-ctypes.rs:56:26
9081
|
9182
LL | pub fn trait_type(p: &dyn Bar);
9283
| ^^^^^^^^ not FFI-safe
9384
|
9485
= note: trait objects have no C equivalent
9586

9687
error: `extern` block uses type `(i32, i32)`, which is not FFI-safe
97-
--> $DIR/lint-ctypes.rs:58:26
88+
--> $DIR/lint-ctypes.rs:57:26
9889
|
9990
LL | pub fn tuple_type(p: (i32, i32));
10091
| ^^^^^^^^^^ not FFI-safe
@@ -103,7 +94,7 @@ LL | pub fn tuple_type(p: (i32, i32));
10394
= note: tuples have unspecified layout
10495

10596
error: `extern` block uses type `(i32, i32)`, which is not FFI-safe
106-
--> $DIR/lint-ctypes.rs:59:27
97+
--> $DIR/lint-ctypes.rs:58:27
10798
|
10899
LL | pub fn tuple_type2(p: I32Pair);
109100
| ^^^^^^^ not FFI-safe
@@ -112,7 +103,7 @@ LL | pub fn tuple_type2(p: I32Pair);
112103
= note: tuples have unspecified layout
113104

114105
error: `extern` block uses type `ZeroSize`, which is not FFI-safe
115-
--> $DIR/lint-ctypes.rs:60:25
106+
--> $DIR/lint-ctypes.rs:59:25
116107
|
117108
LL | pub fn zero_size(p: ZeroSize);
118109
| ^^^^^^^^ not FFI-safe
@@ -126,7 +117,7 @@ LL | pub struct ZeroSize;
126117
| ^^^^^^^^^^^^^^^^^^^
127118

128119
error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe
129-
--> $DIR/lint-ctypes.rs:61:33
120+
--> $DIR/lint-ctypes.rs:60:33
130121
|
131122
LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData);
132123
| ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@@ -139,15 +130,15 @@ LL | pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData<i32>);
139130
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
140131

141132
error: `extern` block uses type `PhantomData<bool>`, which is not FFI-safe
142-
--> $DIR/lint-ctypes.rs:64:12
133+
--> $DIR/lint-ctypes.rs:63:12
143134
|
144135
LL | -> ::std::marker::PhantomData<bool>;
145136
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
146137
|
147138
= note: composed only of `PhantomData`
148139

149140
error: `extern` block uses type `fn()`, which is not FFI-safe
150-
--> $DIR/lint-ctypes.rs:65:23
141+
--> $DIR/lint-ctypes.rs:64:23
151142
|
152143
LL | pub fn fn_type(p: RustFn);
153144
| ^^^^^^ not FFI-safe
@@ -156,7 +147,7 @@ LL | pub fn fn_type(p: RustFn);
156147
= note: this function pointer has Rust-specific calling convention
157148

158149
error: `extern` block uses type `fn()`, which is not FFI-safe
159-
--> $DIR/lint-ctypes.rs:66:24
150+
--> $DIR/lint-ctypes.rs:65:24
160151
|
161152
LL | pub fn fn_type2(p: fn());
162153
| ^^^^ not FFI-safe
@@ -165,7 +156,7 @@ LL | pub fn fn_type2(p: fn());
165156
= note: this function pointer has Rust-specific calling convention
166157

167158
error: `extern` block uses type `Box<u32>`, which is not FFI-safe
168-
--> $DIR/lint-ctypes.rs:67:28
159+
--> $DIR/lint-ctypes.rs:66:28
169160
|
170161
LL | pub fn fn_contained(p: RustBadRet);
171162
| ^^^^^^^^^^ not FFI-safe
@@ -174,7 +165,7 @@ LL | pub fn fn_contained(p: RustBadRet);
174165
= note: this struct has unspecified layout
175166

176167
error: `extern` block uses type `str`, which is not FFI-safe
177-
--> $DIR/lint-ctypes.rs:68:31
168+
--> $DIR/lint-ctypes.rs:67:31
178169
|
179170
LL | pub fn transparent_str(p: TransparentStr);
180171
| ^^^^^^^^^^^^^^ not FFI-safe
@@ -183,7 +174,7 @@ LL | pub fn transparent_str(p: TransparentStr);
183174
= note: string slices have no C equivalent
184175

185176
error: `extern` block uses type `Box<u32>`, which is not FFI-safe
186-
--> $DIR/lint-ctypes.rs:69:30
177+
--> $DIR/lint-ctypes.rs:68:30
187178
|
188179
LL | pub fn transparent_fn(p: TransparentBadFn);
189180
| ^^^^^^^^^^^^^^^^ not FFI-safe
@@ -192,7 +183,7 @@ LL | pub fn transparent_fn(p: TransparentBadFn);
192183
= note: this struct has unspecified layout
193184

194185
error: `extern` block uses type `[u8; 8]`, which is not FFI-safe
195-
--> $DIR/lint-ctypes.rs:70:27
186+
--> $DIR/lint-ctypes.rs:69:27
196187
|
197188
LL | pub fn raw_array(arr: [u8; 8]);
198189
| ^^^^^^^ not FFI-safe
@@ -201,7 +192,7 @@ LL | pub fn raw_array(arr: [u8; 8]);
201192
= note: passing raw arrays by value is not FFI-safe
202193

203194
error: `extern` block uses type `Option<UnsafeCell<extern "C" fn()>>`, which is not FFI-safe
204-
--> $DIR/lint-ctypes.rs:72:26
195+
--> $DIR/lint-ctypes.rs:71:26
205196
|
206197
LL | pub fn no_niche_a(a: Option<UnsafeCell<extern "C" fn()>>);
207198
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@@ -210,13 +201,13 @@ LL | pub fn no_niche_a(a: Option<UnsafeCell<extern "C" fn()>>);
210201
= note: enum has no representation hint
211202

212203
error: `extern` block uses type `Option<UnsafeCell<&i32>>`, which is not FFI-safe
213-
--> $DIR/lint-ctypes.rs:74:26
204+
--> $DIR/lint-ctypes.rs:73:26
214205
|
215206
LL | pub fn no_niche_b(b: Option<UnsafeCell<&i32>>);
216207
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
217208
|
218209
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
219210
= note: enum has no representation hint
220211

221-
error: aborting due to 22 previous errors
212+
error: aborting due to 21 previous errors
222213

0 commit comments

Comments
 (0)