Skip to content

Commit 2f27c6e

Browse files
committed
ImproperCTypes: refactor handling opaque aliases
Put the handling of opaque aliases in the actual `visit___` methods instead of awkwardly pre-checking for them
1 parent 14e4868 commit 2f27c6e

File tree

1 file changed

+58
-21
lines changed

1 file changed

+58
-21
lines changed

compiler/rustc_lint/src/types/improper_ctypes.rs

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,23 @@ use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
2525

2626
type Sig<'tcx> = Binder<'tcx, FnSig<'tcx>>;
2727

28+
// FIXME(ctypes): it seems that tests/ui/lint/opaque-ty-ffi-normalization-cycle.rs relies this:
29+
// we consider opaque aliases that normalise to something else to be unsafe.
30+
// ...is it the behaviour we want?
31+
/// a modified version of cx.tcx.try_normalize_erasing_regions(cx.typing_env(), ty).unwrap_or(ty)
32+
/// so that opaque types prevent normalisation once region erasure occurs
33+
fn erase_and_maybe_normalize<'tcx>(cx: &LateContext<'tcx>, value: Ty<'tcx>) -> Ty<'tcx> {
34+
if (!value.has_aliases()) || value.has_opaque_types() {
35+
cx.tcx.erase_regions(value)
36+
} else {
37+
cx.tcx.try_normalize_erasing_regions(cx.typing_env(), value).unwrap_or(value)
38+
// note: the code above ^^^ would only cause a call to the commented code below vvv
39+
//let value = cx.tcx.erase_regions(value);
40+
//let mut folder = TryNormalizeAfterErasingRegionsFolder::new(cx.tcx, cx.typing_env());
41+
//value.try_fold_with(&mut folder).unwrap_or(value)
42+
}
43+
}
44+
2845
/// Getting the (normalized) type out of a field (for, e.g., an enum variant or a tuple).
2946
#[inline]
3047
fn get_type_from_field<'tcx>(
@@ -33,7 +50,7 @@ fn get_type_from_field<'tcx>(
3350
args: GenericArgsRef<'tcx>,
3451
) -> Ty<'tcx> {
3552
let field_ty = field.ty(cx.tcx, args);
36-
cx.tcx.try_normalize_erasing_regions(cx.typing_env(), field_ty).unwrap_or(field_ty)
53+
erase_and_maybe_normalize(cx, field_ty)
3754
}
3855

3956
/// Check a variant of a non-exhaustive enum for improper ctypes.
@@ -348,7 +365,7 @@ fn get_type_sizedness<'tcx, 'a>(cx: &'a LateContext<'tcx>, ty: Ty<'tcx>) -> Type
348365
Some(item_ty) => *item_ty,
349366
None => bug!("Empty tuple (AKA unit type) should be Sized, right?"),
350367
};
351-
let item_ty = cx.tcx.try_normalize_erasing_regions(cx.typing_env(), item_ty).unwrap_or(item_ty);
368+
let item_ty = erase_and_maybe_normalize(cx, item_ty);
352369
match get_type_sizedness(cx, item_ty) {
353370
s @ (TypeSizedness::UnsizedWithMetadata
354371
| TypeSizedness::UnsizedWithExternType
@@ -1201,25 +1218,52 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12011218

12021219
ty::Never => self.visit_uninhabited(state, ty),
12031220

1204-
// While opaque types are checked for earlier, if a projection in a struct field
1205-
// normalizes to an opaque type, then it will reach this branch.
1221+
// This is only half of the checking-for-opaque-aliases story:
1222+
// since they are liable to vanish on normalisation, we need a specific to find them through
1223+
// other aliases, which is called in the next branch of this `match ty.kind()` statement
12061224
ty::Alias(ty::Opaque, ..) => {
12071225
FfiResult::new_with_reason(ty, fluent::lint_improper_ctypes_opaque, None)
12081226
}
12091227

1210-
// `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe,
1228+
// `extern "C" fn` function definitions can have type parameters, which may or may not be FFI-safe,
12111229
// so they are currently ignored for the purposes of this lint.
1212-
ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..)
1213-
if state.can_expect_ty_params() =>
1214-
{
1215-
FfiSafe
1230+
// function pointers can do the same
1231+
//
1232+
// however, these ty_kind:s can also be encountered because the type isn't normalized yet.
1233+
ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent | ty::Free, ..) => {
1234+
if ty.has_opaque_types() {
1235+
// FIXME(ctypes): this is suboptimal because we give up
1236+
// on reporting anything *else* than the opaque part of the type
1237+
// but this is better than not reporting anything, or crashing
1238+
self.visit_for_opaque_ty(ty)
1239+
} else {
1240+
// in theory, thanks to erase_and_maybe_normalize,
1241+
// normalisation has already occurred
1242+
debug_assert_eq!(
1243+
self.cx
1244+
.tcx
1245+
.try_normalize_erasing_regions(self.cx.typing_env(), ty,)
1246+
.unwrap_or(ty),
1247+
ty,
1248+
);
1249+
1250+
if matches!(
1251+
ty.kind(),
1252+
ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..)
1253+
) && state.can_expect_ty_params()
1254+
{
1255+
FfiSafe
1256+
} else {
1257+
// ty::Alias(ty::Free), and all params/aliases for something
1258+
// defined beyond the FFI boundary
1259+
bug!("unexpected type in foreign function: {:?}", ty)
1260+
}
1261+
}
12161262
}
12171263

12181264
ty::UnsafeBinder(_) => todo!("FIXME(unsafe_binder)"),
12191265

1220-
ty::Param(..)
1221-
| ty::Alias(ty::Projection | ty::Inherent | ty::Free, ..)
1222-
| ty::Infer(..)
1266+
ty::Infer(..)
12231267
| ty::Bound(..)
12241268
| ty::Error(_)
12251269
| ty::Closure(..)
@@ -1257,19 +1301,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12571301
}
12581302

12591303
fn check_for_type(&self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> {
1260-
let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty);
1261-
1262-
match self.visit_for_opaque_ty(ty) {
1263-
FfiResult::FfiSafe => {}
1264-
res @ _ => {
1265-
return res;
1266-
}
1267-
}
1304+
let ty = erase_and_maybe_normalize(self.cx, ty);
12681305
self.visit_type(state, None, ty)
12691306
}
12701307

12711308
fn check_for_fnptr(&self, ty: Ty<'tcx>) -> FfiResult<'tcx> {
1272-
let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty);
1309+
let ty = erase_and_maybe_normalize(self.cx, ty);
12731310

12741311
match *ty.kind() {
12751312
ty::FnPtr(sig_tys, hdr) => {

0 commit comments

Comments
 (0)