Skip to content

Commit fec7a79

Browse files
authored
Rollup merge of rust-lang#94057 - lcnr:simplify_type-uwu, r=nikomatsakis
improve comments for `simplify_type` Should now correctly describe what's going on. Experimented with checking the invariant for projections but that ended up requiring fairly involved changes. I assume that it is not possible to get unsoundness here, at least for now and I can pretty much guarantee that it's impossible to trigger it by accident. r? `````@nikomatsakis````` cc rust-lang#92721
2 parents a638f50 + ba2e0ca commit fec7a79

File tree

7 files changed

+66
-60
lines changed

7 files changed

+66
-60
lines changed

compiler/rustc_metadata/src/rmeta/encoder.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use rustc_middle::mir::interpret;
2626
use rustc_middle::thir;
2727
use rustc_middle::traits::specialization_graph;
2828
use rustc_middle::ty::codec::TyEncoder;
29-
use rustc_middle::ty::fast_reject::{self, SimplifiedType, SimplifyParams};
29+
use rustc_middle::ty::fast_reject::{self, SimplifiedType, TreatParams};
3030
use rustc_middle::ty::query::Providers;
3131
use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt};
3232
use rustc_serialize::{opaque, Encodable, Encoder};
@@ -2043,7 +2043,7 @@ impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplsVisitor<'tcx> {
20432043
let simplified_self_ty = fast_reject::simplify_type(
20442044
self.tcx,
20452045
trait_ref.self_ty(),
2046-
SimplifyParams::No,
2046+
TreatParams::AsPlaceholders,
20472047
);
20482048

20492049
self.impls

compiler/rustc_middle/src/ty/fast_reject.rs

+33-36
Original file line numberDiff line numberDiff line change
@@ -49,36 +49,36 @@ where
4949
}
5050

5151
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
52-
pub enum SimplifyParams {
53-
Yes,
54-
No,
52+
pub enum TreatParams {
53+
/// Treat parameters as bound types in the given environment.
54+
///
55+
/// For this to be correct the input has to be fully normalized
56+
/// in its param env as it may otherwise cause us to ignore
57+
/// potentially applying impls.
58+
AsBoundTypes,
59+
AsPlaceholders,
5560
}
5661

5762
/// Tries to simplify a type by only returning the outermost injective¹ layer, if one exists.
5863
///
5964
/// The idea is to get something simple that we can use to quickly decide if two types could unify,
6065
/// for example during method lookup.
6166
///
62-
/// A special case here are parameters and projections. Projections can be normalized to
63-
/// a different type, meaning that `<T as Trait>::Assoc` and `u8` can be unified, even though
64-
/// their outermost layer is different while parameters like `T` of impls are later replaced
65-
/// with an inference variable, which then also allows unification with other types.
67+
/// A special case here are parameters and projections, which are only injective
68+
/// if they are treated as bound types.
6669
///
67-
/// When using `SimplifyParams::Yes`, we still return a simplified type for params and projections²,
68-
/// the reasoning for this can be seen at the places doing this.
70+
/// For example when storing impls based on their simplified self type, we treat
71+
/// generic parameters as placeholders. We must not simplify them here,
72+
/// as they can unify with any other type.
6973
///
74+
/// With projections we have to be even more careful, as even when treating them as bound types
75+
/// this is still only correct if they are fully normalized.
7076
///
71-
/// ¹ meaning that if two outermost layers are different, then the whole types are also different.
72-
/// ² FIXME(@lcnr): this seems like it can actually end up being unsound with the way it's used during
73-
/// candidate selection. We do not consider non blanket impls for `<_ as Trait>::Assoc` even
74-
/// though `_` can be inferred to a concrete type later at which point a concrete impl
75-
/// could actually apply. After experimenting for about an hour I wasn't able to cause any issues
76-
/// this way so I am not going to change this until we actually find an issue as I am really
77-
/// interesting in getting an actual test for this.
78-
pub fn simplify_type(
79-
tcx: TyCtxt<'_>,
80-
ty: Ty<'_>,
81-
can_simplify_params: SimplifyParams,
77+
/// ¹ meaning that if the outermost layers are different, then the whole types are also different.
78+
pub fn simplify_type<'tcx>(
79+
tcx: TyCtxt<'tcx>,
80+
ty: Ty<'tcx>,
81+
treat_params: TreatParams,
8282
) -> Option<SimplifiedType> {
8383
match *ty.kind() {
8484
ty::Bool => Some(BoolSimplifiedType),
@@ -91,7 +91,7 @@ pub fn simplify_type(
9191
ty::Array(..) => Some(ArraySimplifiedType),
9292
ty::Slice(..) => Some(SliceSimplifiedType),
9393
ty::RawPtr(ptr) => Some(PtrSimplifiedType(ptr.mutbl)),
94-
ty::Dynamic(ref trait_info, ..) => match trait_info.principal_def_id() {
94+
ty::Dynamic(trait_info, ..) => match trait_info.principal_def_id() {
9595
Some(principal_def_id) if !tcx.trait_is_auto(principal_def_id) => {
9696
Some(TraitSimplifiedType(principal_def_id))
9797
}
@@ -100,24 +100,21 @@ pub fn simplify_type(
100100
ty::Ref(_, _, mutbl) => Some(RefSimplifiedType(mutbl)),
101101
ty::FnDef(def_id, _) | ty::Closure(def_id, _) => Some(ClosureSimplifiedType(def_id)),
102102
ty::Generator(def_id, _, _) => Some(GeneratorSimplifiedType(def_id)),
103-
ty::GeneratorWitness(ref tys) => {
104-
Some(GeneratorWitnessSimplifiedType(tys.skip_binder().len()))
105-
}
103+
ty::GeneratorWitness(tys) => Some(GeneratorWitnessSimplifiedType(tys.skip_binder().len())),
106104
ty::Never => Some(NeverSimplifiedType),
107-
ty::Tuple(ref tys) => Some(TupleSimplifiedType(tys.len())),
108-
ty::FnPtr(ref f) => Some(FunctionSimplifiedType(f.skip_binder().inputs().len())),
109-
ty::Projection(_) | ty::Param(_) => {
110-
if can_simplify_params == SimplifyParams::Yes {
111-
// In normalized types, projections don't unify with
112-
// anything. when lazy normalization happens, this
113-
// will change. It would still be nice to have a way
114-
// to deal with known-not-to-unify-with-anything
115-
// projections (e.g., the likes of <__S as Encoder>::Error).
105+
ty::Tuple(tys) => Some(TupleSimplifiedType(tys.len())),
106+
ty::FnPtr(f) => Some(FunctionSimplifiedType(f.skip_binder().inputs().len())),
107+
ty::Param(_) | ty::Projection(_) => match treat_params {
108+
// When treated as bound types, projections don't unify with
109+
// anything as long as they are fully normalized.
110+
//
111+
// We will have to be careful with lazy normalization here.
112+
TreatParams::AsBoundTypes => {
113+
debug!("treating `{}` as a bound type", ty);
116114
Some(ParameterSimplifiedType)
117-
} else {
118-
None
119115
}
120-
}
116+
TreatParams::AsPlaceholders => None,
117+
},
121118
ty::Opaque(def_id, _) => Some(OpaqueSimplifiedType(def_id)),
122119
ty::Foreign(def_id) => Some(ForeignSimplifiedType(def_id)),
123120
ty::Placeholder(..) | ty::Bound(..) | ty::Infer(_) | ty::Error(_) => None,

compiler/rustc_middle/src/ty/trait_def.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::traits::specialization_graph;
2-
use crate::ty::fast_reject::{self, SimplifiedType, SimplifyParams};
2+
use crate::ty::fast_reject::{self, SimplifiedType, TreatParams};
33
use crate::ty::fold::TypeFoldable;
44
use crate::ty::{Ident, Ty, TyCtxt};
55
use rustc_hir as hir;
@@ -150,7 +150,7 @@ impl<'tcx> TyCtxt<'tcx> {
150150
self_ty: Ty<'tcx>,
151151
) -> impl Iterator<Item = DefId> + 'tcx {
152152
let impls = self.trait_impls_of(def_id);
153-
if let Some(simp) = fast_reject::simplify_type(self, self_ty, SimplifyParams::No) {
153+
if let Some(simp) = fast_reject::simplify_type(self, self_ty, TreatParams::AsPlaceholders) {
154154
if let Some(impls) = impls.non_blanket_impls.get(&simp) {
155155
return impls.iter().copied();
156156
}
@@ -180,14 +180,14 @@ impl<'tcx> TyCtxt<'tcx> {
180180
}
181181
}
182182

183-
// Note that we're using `SimplifyParams::Yes` to query `non_blanket_impls` while using
184-
// `SimplifyParams::No` while actually adding them.
183+
// Note that we're using `TreatParams::AsBoundTypes` to query `non_blanket_impls` while using
184+
// `TreatParams::AsPlaceholders` while actually adding them.
185185
//
186186
// This way, when searching for some impl for `T: Trait`, we do not look at any impls
187187
// whose outer level is not a parameter or projection. Especially for things like
188188
// `T: Clone` this is incredibly useful as we would otherwise look at all the impls
189189
// of `Clone` for `Option<T>`, `Vec<T>`, `ConcreteType` and so on.
190-
if let Some(simp) = fast_reject::simplify_type(self, self_ty, SimplifyParams::Yes) {
190+
if let Some(simp) = fast_reject::simplify_type(self, self_ty, TreatParams::AsBoundTypes) {
191191
if let Some(impls) = impls.non_blanket_impls.get(&simp) {
192192
for &impl_def_id in impls {
193193
if let result @ Some(_) = f(impl_def_id) {
@@ -247,7 +247,7 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait
247247
}
248248

249249
if let Some(simplified_self_ty) =
250-
fast_reject::simplify_type(tcx, impl_self_ty, SimplifyParams::No)
250+
fast_reject::simplify_type(tcx, impl_self_ty, TreatParams::AsPlaceholders)
251251
{
252252
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
253253
} else {

compiler/rustc_trait_selection/src/traits/coherence.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use rustc_hir::CRATE_HIR_ID;
2020
use rustc_infer::infer::TyCtxtInferExt;
2121
use rustc_infer::traits::TraitEngine;
2222
use rustc_middle::traits::specialization_graph::OverlapMode;
23-
use rustc_middle::ty::fast_reject::{self, SimplifyParams};
23+
use rustc_middle::ty::fast_reject::{self, TreatParams};
2424
use rustc_middle::ty::fold::TypeFoldable;
2525
use rustc_middle::ty::subst::Subst;
2626
use rustc_middle::ty::{self, Ty, TyCtxt};
@@ -87,8 +87,8 @@ where
8787
impl2_ref.iter().flat_map(|tref| tref.substs.types()),
8888
)
8989
.any(|(ty1, ty2)| {
90-
let t1 = fast_reject::simplify_type(tcx, ty1, SimplifyParams::No);
91-
let t2 = fast_reject::simplify_type(tcx, ty2, SimplifyParams::No);
90+
let t1 = fast_reject::simplify_type(tcx, ty1, TreatParams::AsPlaceholders);
91+
let t2 = fast_reject::simplify_type(tcx, ty2, TreatParams::AsPlaceholders);
9292

9393
if let (Some(t1), Some(t2)) = (t1, t2) {
9494
// Simplified successfully

compiler/rustc_trait_selection/src/traits/select/mod.rs

+9-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use rustc_infer::infer::LateBoundRegionConversionTime;
3636
use rustc_middle::dep_graph::{DepKind, DepNodeIndex};
3737
use rustc_middle::mir::interpret::ErrorHandled;
3838
use rustc_middle::thir::abstract_const::NotConstEvaluatable;
39-
use rustc_middle::ty::fast_reject::{self, SimplifyParams};
39+
use rustc_middle::ty::fast_reject::{self, TreatParams};
4040
use rustc_middle::ty::print::with_no_trimmed_paths;
4141
use rustc_middle::ty::relate::TypeRelation;
4242
use rustc_middle::ty::subst::{GenericArgKind, Subst, SubstsRef};
@@ -2176,8 +2176,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
21762176

21772177
fn fast_reject_trait_refs(
21782178
&mut self,
2179-
obligation: &TraitObligation<'_>,
2180-
impl_trait_ref: &ty::TraitRef<'_>,
2179+
obligation: &TraitObligation<'tcx>,
2180+
impl_trait_ref: &ty::TraitRef<'tcx>,
21812181
) -> bool {
21822182
// We can avoid creating type variables and doing the full
21832183
// substitution if we find that any of the input types, when
@@ -2193,10 +2193,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
21932193
let simplified_obligation_ty = fast_reject::simplify_type(
21942194
self.tcx(),
21952195
obligation_ty,
2196-
SimplifyParams::Yes,
2196+
TreatParams::AsBoundTypes,
2197+
);
2198+
let simplified_impl_ty = fast_reject::simplify_type(
2199+
self.tcx(),
2200+
impl_ty,
2201+
TreatParams::AsPlaceholders,
21972202
);
2198-
let simplified_impl_ty =
2199-
fast_reject::simplify_type(self.tcx(), impl_ty, SimplifyParams::No);
22002203

22012204
simplified_obligation_ty.is_some()
22022205
&& simplified_impl_ty.is_some()

compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::OverlapError;
22

33
use crate::traits;
44
use rustc_hir::def_id::DefId;
5-
use rustc_middle::ty::fast_reject::{self, SimplifiedType, SimplifyParams};
5+
use rustc_middle::ty::fast_reject::{self, SimplifiedType, TreatParams};
66
use rustc_middle::ty::print::with_no_trimmed_paths;
77
use rustc_middle::ty::{self, TyCtxt, TypeFoldable};
88

@@ -49,7 +49,9 @@ impl ChildrenExt<'_> for Children {
4949
/// Insert an impl into this set of children without comparing to any existing impls.
5050
fn insert_blindly(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
5151
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
52-
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), SimplifyParams::No) {
52+
if let Some(st) =
53+
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders)
54+
{
5355
debug!("insert_blindly: impl_def_id={:?} st={:?}", impl_def_id, st);
5456
self.non_blanket_impls.entry(st).or_default().push(impl_def_id)
5557
} else {
@@ -64,7 +66,9 @@ impl ChildrenExt<'_> for Children {
6466
fn remove_existing(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
6567
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
6668
let vec: &mut Vec<DefId>;
67-
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), SimplifyParams::No) {
69+
if let Some(st) =
70+
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders)
71+
{
6872
debug!("remove_existing: impl_def_id={:?} st={:?}", impl_def_id, st);
6973
vec = self.non_blanket_impls.get_mut(&st).unwrap();
7074
} else {
@@ -312,7 +316,8 @@ impl GraphExt for Graph {
312316

313317
let mut parent = trait_def_id;
314318
let mut last_lint = None;
315-
let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), SimplifyParams::No);
319+
let simplified =
320+
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders);
316321

317322
// Descend the specialization tree, where `parent` is the current parent node.
318323
loop {

compiler/rustc_typeck/src/check/method/suggest.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rustc_hir::lang_items::LangItem;
1212
use rustc_hir::{ExprKind, Node, QPath};
1313
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
1414
use rustc_middle::traits::util::supertraits;
15-
use rustc_middle::ty::fast_reject::{simplify_type, SimplifyParams};
15+
use rustc_middle::ty::fast_reject::{simplify_type, TreatParams};
1616
use rustc_middle::ty::print::with_crate_prefix;
1717
use rustc_middle::ty::ToPolyTraitRef;
1818
use rustc_middle::ty::{self, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable};
@@ -1777,7 +1777,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17771777
// FIXME: Even though negative bounds are not implemented, we could maybe handle
17781778
// cases where a positive bound implies a negative impl.
17791779
(candidates, Vec::new())
1780-
} else if let Some(simp_rcvr_ty) = simplify_type(self.tcx, rcvr_ty, SimplifyParams::Yes)
1780+
} else if let Some(simp_rcvr_ty) =
1781+
simplify_type(self.tcx, rcvr_ty, TreatParams::AsBoundTypes)
17811782
{
17821783
let mut potential_candidates = Vec::new();
17831784
let mut explicitly_negative = Vec::new();
@@ -1792,7 +1793,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17921793
.any(|imp_did| {
17931794
let imp = self.tcx.impl_trait_ref(imp_did).unwrap();
17941795
let imp_simp =
1795-
simplify_type(self.tcx, imp.self_ty(), SimplifyParams::Yes);
1796+
simplify_type(self.tcx, imp.self_ty(), TreatParams::AsBoundTypes);
17961797
imp_simp.map_or(false, |s| s == simp_rcvr_ty)
17971798
})
17981799
{

0 commit comments

Comments
 (0)