Skip to content

Commit

Permalink
Auto merge of rust-lang#97024 - lcnr:simplify_type-sus, r=<try>
Browse files Browse the repository at this point in the history
`simplify_type` improvements and cursed docs

the existing `TreatParams` enum pretty much mixes everything up. Not sure why this looked right to me in rust-lang#94057

This also includes two changes which impact perf:
- `ty::Projection` with inference vars shouldn't be treated as a rigid type, even if fully normalized
- `ty::Placeholder` only unifies with itself, so actually return `Some` for them

r? `@nikomatsakis`
  • Loading branch information
bors committed May 19, 2022
2 parents 6913c74 + db19e2b commit c067287
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 86 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1804,7 +1804,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let simplified_self_ty = fast_reject::simplify_type(
self.tcx,
trait_ref.self_ty(),
TreatParams::AsPlaceholders,
TreatParams::AsInfer,
);

fx_hash_map
Expand Down
103 changes: 43 additions & 60 deletions compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
use crate::mir::Mutability;
use crate::ty::{self, Ty, TyCtxt};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use crate::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_hir::def_id::DefId;
use rustc_query_system::ich::StableHashingContext;
use std::fmt::Debug;
use std::hash::Hash;
use std::mem;

use self::SimplifiedTypeGen::*;

Expand All @@ -17,7 +14,7 @@ pub type SimplifiedType = SimplifiedTypeGen<DefId>;
/// because we sometimes need to use SimplifiedTypeGen values as stable sorting
/// keys (in which case we use a DefPathHash as id-type) but in the general case
/// the non-stable but fast to construct DefId-version is the better choice.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, TyEncodable, TyDecodable)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, TyEncodable, TyDecodable, HashStable)]
pub enum SimplifiedTypeGen<D>
where
D: Copy + Debug + Eq,
Expand Down Expand Up @@ -45,34 +42,49 @@ where
GeneratorWitnessSimplifiedType(usize),
OpaqueSimplifiedType(D),
FunctionSimplifiedType(usize),
ParameterSimplifiedType,
PlaceholderSimplifiedType,
}

/// Generic parameters are pretty much just bound variables, e.g.
/// the type of `fn foo<'a, T>(x: &'a T) -> u32 { ... }` can be thought of as
/// `for<'a, T> fn(&'a T) -> u32`.
///
/// Typecheck of `foo` has to succeed for all possible generic arguments, so
/// during typeck, we have to treat its generic parameters as if they
/// were placeholders.
///
/// But when calling `foo` we only have to provide a specific generic argument.
/// In that case the generic parameters are instantiated with inference variables.
/// As we use `simplify_type` before that instantiation happens, we just treat
/// generic parameters as if they were inference variables in that case.
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum TreatParams {
/// Treat parameters as bound types in the given environment.
/// Treat parameters as placeholders in the given environment.
///
/// For this to be correct the input has to be fully normalized
/// in its param env as it may otherwise cause us to ignore
/// potentially applying impls.
AsBoundTypes,
AsPlaceholders,
/// Note that this also causes us to treat projections as if they were
/// placeholders. This is only correct if the given projection cannot
/// be normalized in the current context. Even if normalization fails,
/// it may still succeed later if the projection contains any inference
/// variables.
AsPlaceholder,
AsInfer,
}

/// Tries to simplify a type by only returning the outermost injective¹ layer, if one exists.
///
/// The idea is to get something simple that we can use to quickly decide if two types could unify,
/// for example during method lookup.
/// for example during method lookup. If this function returns `Some(x)` it can only unify with
/// types for which this method returns either `Some(x)` as well or `None`.
///
/// A special case here are parameters and projections, which are only injective
/// if they are treated as bound types.
/// if they are treated as placeholders.
///
/// For example when storing impls based on their simplified self type, we treat
/// generic parameters as placeholders. We must not simplify them here,
/// generic parameters as if they were inference variables. We must not simplify them here,
/// as they can unify with any other type.
///
/// With projections we have to be even more careful, as even when treating them as bound types
/// this is still only correct if they are fully normalized.
/// With projections we have to be even more careful, as treating them as placeholders
/// is only correct if they are fully normalized.
///
/// ¹ meaning that if the outermost layers are different, then the whole types are also different.
pub fn simplify_type<'tcx>(
Expand Down Expand Up @@ -104,20 +116,25 @@ pub fn simplify_type<'tcx>(
ty::Never => Some(NeverSimplifiedType),
ty::Tuple(tys) => Some(TupleSimplifiedType(tys.len())),
ty::FnPtr(f) => Some(FunctionSimplifiedType(f.skip_binder().inputs().len())),
ty::Param(_) | ty::Projection(_) => match treat_params {
// When treated as bound types, projections don't unify with
// anything as long as they are fully normalized.
ty::Placeholder(..) => Some(PlaceholderSimplifiedType),
ty::Param(_) => match treat_params {
TreatParams::AsPlaceholder => Some(PlaceholderSimplifiedType),
TreatParams::AsInfer => None,
},
ty::Projection(_) => match treat_params {
// When treating `ty::Param` as a placeholder, projections also
// don't unify with anything else as long as they are fully normalized.
//
// We will have to be careful with lazy normalization here.
TreatParams::AsBoundTypes => {
debug!("treating `{}` as a bound type", ty);
Some(ParameterSimplifiedType)
TreatParams::AsPlaceholder if !ty.has_infer_types_or_consts() => {
debug!("treating `{}` as a placeholder", ty);
Some(PlaceholderSimplifiedType)
}
TreatParams::AsPlaceholders => None,
TreatParams::AsPlaceholder | TreatParams::AsInfer => None,
},
ty::Opaque(def_id, _) => Some(OpaqueSimplifiedType(def_id)),
ty::Foreign(def_id) => Some(ForeignSimplifiedType(def_id)),
ty::Placeholder(..) | ty::Bound(..) | ty::Infer(_) | ty::Error(_) => None,
ty::Bound(..) | ty::Infer(_) | ty::Error(_) => None,
}
}

Expand Down Expand Up @@ -161,41 +178,7 @@ impl<D: Copy + Debug + Eq> SimplifiedTypeGen<D> {
GeneratorWitnessSimplifiedType(n) => GeneratorWitnessSimplifiedType(n),
OpaqueSimplifiedType(d) => OpaqueSimplifiedType(map(d)),
FunctionSimplifiedType(n) => FunctionSimplifiedType(n),
ParameterSimplifiedType => ParameterSimplifiedType,
}
}
}

impl<'a, D> HashStable<StableHashingContext<'a>> for SimplifiedTypeGen<D>
where
D: Copy + Debug + Eq + HashStable<StableHashingContext<'a>>,
{
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
BoolSimplifiedType
| CharSimplifiedType
| StrSimplifiedType
| ArraySimplifiedType
| SliceSimplifiedType
| NeverSimplifiedType
| ParameterSimplifiedType
| MarkerTraitObjectSimplifiedType => {
// nothing to do
}
RefSimplifiedType(m) | PtrSimplifiedType(m) => m.hash_stable(hcx, hasher),
IntSimplifiedType(t) => t.hash_stable(hcx, hasher),
UintSimplifiedType(t) => t.hash_stable(hcx, hasher),
FloatSimplifiedType(t) => t.hash_stable(hcx, hasher),
AdtSimplifiedType(d) => d.hash_stable(hcx, hasher),
TupleSimplifiedType(n) => n.hash_stable(hcx, hasher),
TraitSimplifiedType(d) => d.hash_stable(hcx, hasher),
ClosureSimplifiedType(d) => d.hash_stable(hcx, hasher),
GeneratorSimplifiedType(d) => d.hash_stable(hcx, hasher),
GeneratorWitnessSimplifiedType(n) => n.hash_stable(hcx, hasher),
OpaqueSimplifiedType(d) => d.hash_stable(hcx, hasher),
FunctionSimplifiedType(n) => n.hash_stable(hcx, hasher),
ForeignSimplifiedType(d) => d.hash_stable(hcx, hasher),
PlaceholderSimplifiedType => PlaceholderSimplifiedType,
}
}
}
10 changes: 5 additions & 5 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<'tcx> TyCtxt<'tcx> {
self_ty: Ty<'tcx>,
) -> impl Iterator<Item = DefId> + 'tcx {
let impls = self.trait_impls_of(def_id);
if let Some(simp) = fast_reject::simplify_type(self, self_ty, TreatParams::AsPlaceholders) {
if let Some(simp) = fast_reject::simplify_type(self, self_ty, TreatParams::AsInfer) {
if let Some(impls) = impls.non_blanket_impls.get(&simp) {
return impls.iter().copied();
}
Expand Down Expand Up @@ -173,14 +173,14 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

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

if let Some(simplified_self_ty) =
fast_reject::simplify_type(tcx, impl_self_ty, TreatParams::AsPlaceholders)
fast_reject::simplify_type(tcx, impl_self_ty, TreatParams::AsInfer)
{
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
} else {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ where
impl2_ref.iter().flat_map(|tref| tref.substs.types()),
)
.any(|(ty1, ty2)| {
let t1 = fast_reject::simplify_type(tcx, ty1, TreatParams::AsPlaceholders);
let t2 = fast_reject::simplify_type(tcx, ty2, TreatParams::AsPlaceholders);
let t1 = fast_reject::simplify_type(tcx, ty1, TreatParams::AsInfer);
let t2 = fast_reject::simplify_type(tcx, ty2, TreatParams::AsInfer);

if let (Some(t1), Some(t2)) = (t1, t2) {
// Simplified successfully
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2148,13 +2148,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let simplified_obligation_ty = fast_reject::simplify_type(
self.tcx(),
obligation_ty,
TreatParams::AsBoundTypes,
);
let simplified_impl_ty = fast_reject::simplify_type(
self.tcx(),
impl_ty,
TreatParams::AsPlaceholders,
TreatParams::AsPlaceholder,
);
let simplified_impl_ty =
fast_reject::simplify_type(self.tcx(), impl_ty, TreatParams::AsInfer);

simplified_obligation_ty.is_some()
&& simplified_impl_ty.is_some()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ impl ChildrenExt<'_> for Children {
/// Insert an impl into this set of children without comparing to any existing impls.
fn insert_blindly(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
if let Some(st) =
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders)
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsInfer)
{
debug!("insert_blindly: impl_def_id={:?} st={:?}", impl_def_id, st);
self.non_blanket_impls.entry(st).or_default().push(impl_def_id)
Expand All @@ -66,8 +65,7 @@ impl ChildrenExt<'_> for Children {
fn remove_existing(&mut self, tcx: TyCtxt<'_>, impl_def_id: DefId) {
let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
let vec: &mut Vec<DefId>;
if let Some(st) =
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders)
if let Some(st) = fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsInfer)
{
debug!("remove_existing: impl_def_id={:?} st={:?}", impl_def_id, st);
vec = self.non_blanket_impls.get_mut(&st).unwrap();
Expand Down Expand Up @@ -316,8 +314,7 @@ impl GraphExt for Graph {

let mut parent = trait_def_id;
let mut last_lint = None;
let simplified =
fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsPlaceholders);
let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), TreatParams::AsInfer);

// Descend the specialization tree, where `parent` is the current parent node.
loop {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}

fn assemble_inherent_candidates_for_incoherent_ty(&mut self, self_ty: Ty<'tcx>) {
let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::AsPlaceholders) else {
let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::AsInfer) else {
bug!("unexpected incoherent type: {:?}", self_ty)
};
for &impl_def_id in self.tcx.incoherent_impls(simp) {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_typeck/src/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.into_iter()
.any(|info| self.associated_value(info.def_id, item_name).is_some());
let found_assoc = |ty: Ty<'tcx>| {
simplify_type(tcx, ty, TreatParams::AsPlaceholders)
simplify_type(tcx, ty, TreatParams::AsInfer)
.and_then(|simp| {
tcx.incoherent_impls(simp)
.iter()
Expand Down Expand Up @@ -1956,7 +1956,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// cases where a positive bound implies a negative impl.
(candidates, Vec::new())
} else if let Some(simp_rcvr_ty) =
simplify_type(self.tcx, rcvr_ty, TreatParams::AsBoundTypes)
simplify_type(self.tcx, rcvr_ty, TreatParams::AsPlaceholder)
{
let mut potential_candidates = Vec::new();
let mut explicitly_negative = Vec::new();
Expand All @@ -1971,7 +1971,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.any(|imp_did| {
let imp = self.tcx.impl_trait_ref(imp_did).unwrap();
let imp_simp =
simplify_type(self.tcx, imp.self_ty(), TreatParams::AsBoundTypes);
simplify_type(self.tcx, imp.self_ty(), TreatParams::AsPlaceholder);
imp_simp.map_or(false, |s| s == simp_rcvr_ty)
})
{
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl<'tcx> InherentCollect<'tcx> {
}
}

if let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::AsPlaceholders) {
if let Some(simp) = simplify_type(self.tcx, self_ty, TreatParams::AsInfer) {
self.impls_map.incoherent_impls.entry(simp).or_default().push(impl_def_id);
} else {
bug!("unexpected self type: {:?}", self_ty);
Expand Down Expand Up @@ -169,7 +169,7 @@ impl<'tcx> InherentCollect<'tcx> {
}
}

if let Some(simp) = simplify_type(self.tcx, ty, TreatParams::AsPlaceholders) {
if let Some(simp) = simplify_type(self.tcx, ty, TreatParams::AsInfer) {
self.impls_map.incoherent_impls.entry(simp).or_default().push(impl_def_id);
} else {
bug!("unexpected primitive type: {:?}", ty);
Expand Down

0 comments on commit c067287

Please sign in to comment.