Skip to content

Commit d3ce4dd

Browse files
committed
Remove is_normalizable. Add sizedness_of to avoid layout_of query on type aliases.
1 parent 1c81105 commit d3ce4dd

File tree

6 files changed

+138
-63
lines changed

6 files changed

+138
-63
lines changed

clippy_lints/src/transmute/eager_transmute.rs

-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::ty::is_normalizable;
32
use clippy_utils::{eq_expr_value, path_to_local};
43
use rustc_abi::WrappingRange;
54
use rustc_errors::Applicability;
@@ -84,8 +83,6 @@ pub(super) fn check<'tcx>(
8483
&& path.ident.name == sym!(then_some)
8584
&& is_local_with_projections(transmutable)
8685
&& binops_with_local(cx, transmutable, receiver)
87-
&& is_normalizable(cx, cx.param_env, from_ty)
88-
&& is_normalizable(cx, cx.param_env, to_ty)
8986
// we only want to lint if the target type has a niche that is larger than the one of the source type
9087
// e.g. `u8` to `NonZero<u8>` should lint, but `NonZero<u8>` to `u8` should not
9188
&& let Ok(from_layout) = cx.tcx.layout_of(cx.param_env.and(from_ty))

clippy_lints/src/zero_sized_map_values.rs

+3-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use clippy_utils::ty::{is_normalizable, is_type_diagnostic_item};
2+
use clippy_utils::ty::{is_type_diagnostic_item, sizedness_of};
33
use rustc_hir::{self as hir, HirId, ItemKind, Node};
44
use rustc_hir_analysis::lower_ty;
55
use rustc_lint::{LateContext, LateLintPass};
6-
use rustc_middle::ty::layout::LayoutOf as _;
7-
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
6+
use rustc_middle::ty::{self, Ty};
87
use rustc_session::declare_lint_pass;
98
use rustc_span::sym;
109

@@ -51,13 +50,7 @@ impl LateLintPass<'_> for ZeroSizedMapValues {
5150
&& (is_type_diagnostic_item(cx, ty, sym::HashMap) || is_type_diagnostic_item(cx, ty, sym::BTreeMap))
5251
&& let ty::Adt(_, args) = ty.kind()
5352
&& let ty = args.type_at(1)
54-
// Fixes https://github.com/rust-lang/rust-clippy/issues/7447 because of
55-
// https://github.com/rust-lang/rust/blob/master/compiler/rustc_middle/src/ty/sty.rs#L968
56-
&& !ty.has_escaping_bound_vars()
57-
// Do this to prevent `layout_of` crashing, being unable to fully normalize `ty`.
58-
&& is_normalizable(cx, cx.param_env, ty)
59-
&& let Ok(layout) = cx.layout_of(ty)
60-
&& layout.is_zst()
53+
&& sizedness_of(cx.tcx, cx.param_env, ty).is_zero()
6154
{
6255
span_lint_and_help(
6356
cx,

clippy_utils/src/ty.rs

+94-49
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_lint::LateContext;
1515
use rustc_middle::mir::interpret::Scalar;
1616
use rustc_middle::mir::ConstValue;
1717
use rustc_middle::traits::EvaluationResult;
18-
use rustc_middle::ty::layout::ValidityRequirement;
18+
use rustc_middle::ty::layout::{LayoutOf, ValidityRequirement};
1919
use rustc_middle::ty::{
2020
self, AdtDef, AliasTy, AssocItem, AssocKind, Binder, BoundRegion, FnSig, GenericArg, GenericArgKind,
2121
GenericArgsRef, GenericParamDefKind, IntTy, ParamEnv, Region, RegionKind, TraitRef, Ty, TyCtxt, TypeSuperVisitable,
@@ -353,50 +353,6 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
353353
}
354354
}
355355

356-
// FIXME: Per https://doc.rust-lang.org/nightly/nightly-rustc/rustc_trait_selection/infer/at/struct.At.html#method.normalize
357-
// this function can be removed once the `normalize` method does not panic when normalization does
358-
// not succeed
359-
/// Checks if `Ty` is normalizable. This function is useful
360-
/// to avoid crashes on `layout_of`.
361-
pub fn is_normalizable<'tcx>(cx: &LateContext<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
362-
is_normalizable_helper(cx, param_env, ty, &mut FxHashMap::default())
363-
}
364-
365-
fn is_normalizable_helper<'tcx>(
366-
cx: &LateContext<'tcx>,
367-
param_env: ParamEnv<'tcx>,
368-
ty: Ty<'tcx>,
369-
cache: &mut FxHashMap<Ty<'tcx>, bool>,
370-
) -> bool {
371-
if let Some(&cached_result) = cache.get(&ty) {
372-
return cached_result;
373-
}
374-
// prevent recursive loops, false-negative is better than endless loop leading to stack overflow
375-
cache.insert(ty, false);
376-
let infcx = cx.tcx.infer_ctxt().build();
377-
let cause = ObligationCause::dummy();
378-
let result = if infcx.at(&cause, param_env).query_normalize(ty).is_ok() {
379-
match ty.kind() {
380-
ty::Adt(def, args) => def.variants().iter().all(|variant| {
381-
variant
382-
.fields
383-
.iter()
384-
.all(|field| is_normalizable_helper(cx, param_env, field.ty(cx.tcx, args), cache))
385-
}),
386-
_ => ty.walk().all(|generic_arg| match generic_arg.unpack() {
387-
GenericArgKind::Type(inner_ty) if inner_ty != ty => {
388-
is_normalizable_helper(cx, param_env, inner_ty, cache)
389-
},
390-
_ => true, // if inner_ty == ty, we've already checked it
391-
}),
392-
}
393-
} else {
394-
false
395-
};
396-
cache.insert(ty, result);
397-
result
398-
}
399-
400356
/// Returns `true` if the given type is a non aggregate primitive (a `bool` or `char`, any
401357
/// integer or floating-point number type). For checking aggregation of primitive types (e.g.
402358
/// tuples and slices of primitive type) see `is_recursively_primitive_type`
@@ -977,11 +933,12 @@ pub fn adt_and_variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option<
977933

978934
/// Comes up with an "at least" guesstimate for the type's size, not taking into
979935
/// account the layout of type parameters.
936+
///
937+
/// This function will ICE if called with an improper `ParamEnv`. This can happen
938+
/// when linting in when item, but the type is retrieved from a different item
939+
/// without instantiating the generic arguments. It can also happen when linting a
940+
/// type alias as those do not have a `ParamEnv`.
980941
pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
981-
use rustc_middle::ty::layout::LayoutOf;
982-
if !is_normalizable(cx, cx.param_env, ty) {
983-
return 0;
984-
}
985942
match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) {
986943
(Ok(size), _) => size,
987944
(Err(_), ty::Tuple(list)) => list.iter().map(|t| approx_ty_size(cx, t)).sum(),
@@ -1340,3 +1297,91 @@ pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) ->
13401297
_ => None,
13411298
}
13421299
}
1300+
1301+
#[derive(Clone, Copy)]
1302+
pub enum Sizedness {
1303+
/// The type is uninhabited. (e.g. `!`)
1304+
Uninhabited,
1305+
/// The type is zero-sized.
1306+
Zero,
1307+
/// The type has some other size or an unknown size.
1308+
Other,
1309+
}
1310+
impl Sizedness {
1311+
pub fn is_zero(self) -> bool {
1312+
matches!(self, Self::Zero)
1313+
}
1314+
1315+
pub fn is_uninhabited(self) -> bool {
1316+
matches!(self, Self::Uninhabited)
1317+
}
1318+
}
1319+
1320+
/// Calculates the sizedness of a type.
1321+
pub fn sizedness_of<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> Sizedness {
1322+
fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
1323+
match *ty.kind() {
1324+
ty::FnDef(..) | ty::Never => true,
1325+
ty::Tuple(tys) => tys.iter().all(|ty| is_zst(tcx, param_env, ty)),
1326+
// Zero length arrays are always zero-sized, even for uninhabited types.
1327+
ty::Array(_, len) if len.try_eval_target_usize(tcx, param_env).is_some_and(|x| x == 0) => true,
1328+
ty::Array(ty, _) | ty::Pat(ty, _) => is_zst(tcx, param_env, ty),
1329+
ty::Adt(adt, args) => {
1330+
let mut iter = adt.variants().iter().filter(|v| {
1331+
!v.fields
1332+
.iter()
1333+
.any(|f| f.ty(tcx, args).is_privately_uninhabited(tcx, param_env))
1334+
});
1335+
let is_zst = iter.next().map_or(true, |v| {
1336+
v.fields.iter().all(|f| is_zst(tcx, param_env, f.ty(tcx, args)))
1337+
});
1338+
is_zst && iter.next().is_none()
1339+
},
1340+
ty::Closure(_, args) => args
1341+
.as_closure()
1342+
.upvar_tys()
1343+
.iter()
1344+
.all(|ty| is_zst(tcx, param_env, ty)),
1345+
ty::CoroutineWitness(_, args) => args
1346+
.iter()
1347+
.filter_map(GenericArg::as_type)
1348+
.all(|ty| is_zst(tcx, param_env, ty)),
1349+
ty::Alias(..) => {
1350+
if let Ok(normalized) = tcx.try_normalize_erasing_regions(param_env, ty)
1351+
&& normalized != ty
1352+
{
1353+
is_zst(tcx, param_env, normalized)
1354+
} else {
1355+
false
1356+
}
1357+
},
1358+
ty::Bool
1359+
| ty::Char
1360+
| ty::Int(_)
1361+
| ty::Uint(_)
1362+
| ty::Float(_)
1363+
| ty::RawPtr(..)
1364+
| ty::Ref(..)
1365+
| ty::FnPtr(..)
1366+
| ty::Param(_)
1367+
| ty::Bound(..)
1368+
| ty::Placeholder(_)
1369+
| ty::Infer(_)
1370+
| ty::Error(_)
1371+
| ty::Dynamic(..)
1372+
| ty::Slice(..)
1373+
| ty::Str
1374+
| ty::Foreign(_)
1375+
| ty::Coroutine(..)
1376+
| ty::CoroutineClosure(..) => false,
1377+
}
1378+
}
1379+
1380+
if ty.is_privately_uninhabited(tcx, param_env) {
1381+
Sizedness::Uninhabited
1382+
} else if is_zst(tcx, param_env, ty) {
1383+
Sizedness::Zero
1384+
} else {
1385+
Sizedness::Other
1386+
}
1387+
}

tests/ui/crashes/ice-10508.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Used to overflow in `is_normalizable`
2+
3+
use std::marker::PhantomData;
4+
5+
struct Node<T: 'static> {
6+
m: PhantomData<&'static T>,
7+
}
8+
9+
struct Digit<T> {
10+
elem: T,
11+
}
12+
13+
enum FingerTree<T: 'static> {
14+
Single(T),
15+
16+
Deep(Digit<T>, Box<FingerTree<Node<T>>>),
17+
}
18+
19+
fn main() {}

tests/ui/large_enum_variant.64bit.stderr

+17-1
Original file line numberDiff line numberDiff line change
@@ -276,5 +276,21 @@ help: consider boxing the large fields to reduce the total size of the enum
276276
LL | Error(Box<PossiblyLargeEnumWithConst<256>>),
277277
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
278278

279-
error: aborting due to 16 previous errors
279+
error: large size difference between variants
280+
--> tests/ui/large_enum_variant.rs:167:1
281+
|
282+
LL | / enum SelfRef<'a> {
283+
LL | | Small,
284+
| | ----- the second-largest variant carries no data at all
285+
LL | | Large([&'a SelfRef<'a>; 1024]),
286+
| | ------------------------------ the largest variant contains at least 8192 bytes
287+
LL | | }
288+
| |_^ the entire enum is at least 8192 bytes
289+
|
290+
help: consider boxing the large fields to reduce the total size of the enum
291+
|
292+
LL | Large(Box<[&'a SelfRef<'a>; 1024]>),
293+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
294+
295+
error: aborting due to 17 previous errors
280296

tests/ui/large_enum_variant.rs

+5
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,8 @@ fn main() {
163163
}
164164
);
165165
}
166+
167+
enum SelfRef<'a> {
168+
Small,
169+
Large([&'a SelfRef<'a>; 1024]),
170+
}

0 commit comments

Comments
 (0)