Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

extend simplify_type #86986

Merged
merged 5 commits into from
Dec 16, 2021
Merged

extend simplify_type #86986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dcd716f
extend `simplify_type`
lcnr May 7, 2021
992efa6
replace `DefId` with `D` in `SimplifiedType`
lcnr Jul 12, 2021
023b565
add some comments
lcnr Nov 19, 2021
095ad18
fmt
lcnr Dec 14, 2021
00cbacb
update tests
lcnr Dec 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use rustc_middle::mir::interpret;
use rustc_middle::thir;
use rustc_middle::traits::specialization_graph;
use rustc_middle::ty::codec::TyEncoder;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt};
use rustc_serialize::{opaque, Encodable, Encoder};
use rustc_session::config::CrateType;
Expand Down Expand Up @@ -2033,15 +2034,19 @@ impl EncodeContext<'a, 'tcx> {

struct ImplVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
impls: FxHashMap<DefId, Vec<(DefIndex, Option<ty::fast_reject::SimplifiedType>)>>,
impls: FxHashMap<DefId, Vec<(DefIndex, Option<fast_reject::SimplifiedType>)>>,
}

impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplVisitor<'tcx> {
fn visit_item(&mut self, item: &hir::Item<'_>) {
if let hir::ItemKind::Impl { .. } = item.kind {
if let Some(trait_ref) = self.tcx.impl_trait_ref(item.def_id.to_def_id()) {
let simplified_self_ty =
ty::fast_reject::simplify_type(self.tcx, trait_ref.self_ty(), false);
let simplified_self_ty = fast_reject::simplify_type(
self.tcx,
trait_ref.self_ty(),
SimplifyParams::No,
StripReferences::No,
);

self.impls
.entry(trait_ref.def_id)
Expand Down
82 changes: 60 additions & 22 deletions compiler/rustc_middle/src/ty/fast_reject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::mir::Mutability;
use crate::ty::{self, Ty, TyCtxt};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -27,9 +28,12 @@ where
UintSimplifiedType(ty::UintTy),
FloatSimplifiedType(ty::FloatTy),
AdtSimplifiedType(D),
ForeignSimplifiedType(D),
StrSimplifiedType,
ArraySimplifiedType,
PtrSimplifiedType,
SliceSimplifiedType,
RefSimplifiedType(Mutability),
PtrSimplifiedType(Mutability),
NeverSimplifiedType,
TupleSimplifiedType(usize),
/// A trait object, all of whose components are markers
Expand All @@ -42,22 +46,48 @@ where
OpaqueSimplifiedType(D),
FunctionSimplifiedType(usize),
ParameterSimplifiedType,
ForeignSimplifiedType(DefId),
}

/// Tries to simplify a type by dropping type parameters, deref'ing away any reference types, etc.
/// The idea is to get something simple that we can use to quickly decide if two types could unify
/// during method lookup.
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum SimplifyParams {
Yes,
No,
}

#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub enum StripReferences {
lcnr marked this conversation as resolved.
Show resolved Hide resolved
Yes,
No,
}

/// 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.
///
/// If `can_simplify_params` is false, then we will fail to simplify type parameters entirely. This
/// is useful when those type parameters would be instantiated with fresh type variables, since
/// then we can't say much about whether two types would unify. Put another way,
/// `can_simplify_params` should be true if type parameters appear free in `ty` and `false` if they
/// are to be considered bound.
/// A special case here are parameters and projections. Projections can be normalized to
/// a different type, meaning that `<T as Trait>::Assoc` and `u8` can be unified, even though
/// their outermost layer is different while parameters like `T` of impls are later replaced
/// with an inference variable, which then also allows unification with other types.
///
/// When using `SimplifyParams::Yes`, we still return a simplified type for params and projections²,
/// the reasoning for this can be seen at the places doing this.
///
/// For diagnostics we strip references with `StripReferences::Yes`. This is currently the best
/// way to skip some unhelpful suggestions.
///
/// ¹ meaning that if two outermost layers are different, then the whole types are also different.
/// ² FIXME(@lcnr): this seems like it can actually end up being unsound with the way it's used during
/// candidate selection. We do not consider non blanket impls for `<_ as Trait>::Assoc` even
/// though `_` can be inferred to a concrete type later at which point a concrete impl
/// could actually apply. After experimenting for about an hour I wasn't able to cause any issues
/// this way so I am not going to change this until we actually find an issue as I am really
/// interesting in getting an actual test for this.
Comment on lines +80 to +85
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis does this comment make sense to you? I hoped to trigger it by using something like the following, but wasn't able to make much progress. We either normalize the projection because it is already concrete enough or fail for some different reason. Not sure if this is actually exploitable if it even is an issue.

trait Id {
    type Assoc;
}

impl<T> Id for T {
    type Assoc = Self;
}

trait Bar<T> {
    type Assoc;
    fn bar(self) -> Self::Assoc; 
}

impl Bar<u32> for i32 {
    type Assoc = i32;
    fn bar(self) -> Self::Assoc {
        1
    }
}

fn main() {
    let mut x = <<_ as Id>::Assoc>::default();
    x = <_ as Bar<u32>>::bar(x);
}

pub fn simplify_type(
tcx: TyCtxt<'_>,
ty: Ty<'_>,
can_simplify_params: bool,
can_simplify_params: SimplifyParams,
strip_references: StripReferences,
) -> Option<SimplifiedType> {
match *ty.kind() {
ty::Bool => Some(BoolSimplifiedType),
Expand All @@ -67,19 +97,24 @@ pub fn simplify_type(
ty::Float(float_type) => Some(FloatSimplifiedType(float_type)),
ty::Adt(def, _) => Some(AdtSimplifiedType(def.did)),
ty::Str => Some(StrSimplifiedType),
ty::Array(..) | ty::Slice(_) => Some(ArraySimplifiedType),
ty::RawPtr(_) => Some(PtrSimplifiedType),
ty::Array(..) => Some(ArraySimplifiedType),
ty::Slice(..) => Some(SliceSimplifiedType),
ty::RawPtr(ptr) => Some(PtrSimplifiedType(ptr.mutbl)),
ty::Dynamic(ref trait_info, ..) => match trait_info.principal_def_id() {
Some(principal_def_id) if !tcx.trait_is_auto(principal_def_id) => {
Some(TraitSimplifiedType(principal_def_id))
}
_ => Some(MarkerTraitObjectSimplifiedType),
},
ty::Ref(_, ty, _) => {
// since we introduce auto-refs during method lookup, we
// just treat &T and T as equivalent from the point of
// view of possibly unifying
simplify_type(tcx, ty, can_simplify_params)
ty::Ref(_, ty, mutbl) => {
if strip_references == StripReferences::Yes {
// For diagnostics, when recommending similar impls we want to
// recommend impls even when there is a reference mismatch,
// so we treat &T and T equivalently in that case.
simplify_type(tcx, ty, can_simplify_params, strip_references)
} else {
Some(RefSimplifiedType(mutbl))
}
}
ty::FnDef(def_id, _) | ty::Closure(def_id, _) => Some(ClosureSimplifiedType(def_id)),
ty::Generator(def_id, _, _) => Some(GeneratorSimplifiedType(def_id)),
Expand All @@ -90,7 +125,7 @@ pub fn simplify_type(
ty::Tuple(ref tys) => Some(TupleSimplifiedType(tys.len())),
ty::FnPtr(ref f) => Some(FunctionSimplifiedType(f.skip_binder().inputs().len())),
ty::Projection(_) | ty::Param(_) => {
if can_simplify_params {
if can_simplify_params == SimplifyParams::Yes {
// In normalized types, projections don't unify with
// anything. when lazy normalization happens, this
// will change. It would still be nice to have a way
Expand Down Expand Up @@ -120,9 +155,12 @@ impl<D: Copy + Debug + Ord + Eq> SimplifiedTypeGen<D> {
UintSimplifiedType(t) => UintSimplifiedType(t),
FloatSimplifiedType(t) => FloatSimplifiedType(t),
AdtSimplifiedType(d) => AdtSimplifiedType(map(d)),
ForeignSimplifiedType(d) => ForeignSimplifiedType(map(d)),
StrSimplifiedType => StrSimplifiedType,
ArraySimplifiedType => ArraySimplifiedType,
PtrSimplifiedType => PtrSimplifiedType,
SliceSimplifiedType => SliceSimplifiedType,
RefSimplifiedType(m) => RefSimplifiedType(m),
PtrSimplifiedType(m) => PtrSimplifiedType(m),
NeverSimplifiedType => NeverSimplifiedType,
MarkerTraitObjectSimplifiedType => MarkerTraitObjectSimplifiedType,
TupleSimplifiedType(n) => TupleSimplifiedType(n),
Expand All @@ -133,7 +171,6 @@ impl<D: Copy + Debug + Ord + Eq> SimplifiedTypeGen<D> {
OpaqueSimplifiedType(d) => OpaqueSimplifiedType(map(d)),
FunctionSimplifiedType(n) => FunctionSimplifiedType(n),
ParameterSimplifiedType => ParameterSimplifiedType,
ForeignSimplifiedType(d) => ForeignSimplifiedType(d),
}
}
}
Expand All @@ -149,12 +186,13 @@ where
| CharSimplifiedType
| StrSimplifiedType
| ArraySimplifiedType
| PtrSimplifiedType
| 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),
Expand Down
45 changes: 18 additions & 27 deletions compiler/rustc_middle/src/ty/trait_def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::traits::specialization_graph;
use crate::ty::fast_reject;
use crate::ty::fast_reject::{self, SimplifyParams, StripReferences};
use crate::ty::fold::TypeFoldable;
use crate::ty::{Ty, TyCtxt};
use rustc_hir as hir;
Expand Down Expand Up @@ -146,6 +146,11 @@ impl<'tcx> TyCtxt<'tcx> {
self_ty: Ty<'tcx>,
mut f: F,
) -> Option<T> {
// FIXME: This depends on the set of all impls for the trait. That is
// unfortunate wrt. incremental compilation.
//
// If we want to be faster, we could have separate queries for
// blanket and non-blanket impls, and compare them separately.
let impls = self.trait_impls_of(def_id);

for &impl_def_id in impls.blanket_impls.iter() {
Expand All @@ -154,32 +159,16 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

// simplify_type(.., false) basically replaces type parameters and
// projections with infer-variables. This is, of course, done on
// the impl trait-ref when it is instantiated, but not on the
// predicate trait-ref which is passed here.
//
// for example, if we match `S: Copy` against an impl like
// `impl<T:Copy> Copy for Option<T>`, we replace the type variable
// in `Option<T>` with an infer variable, to `Option<_>` (this
// doesn't actually change fast_reject output), but we don't
// replace `S` with anything - this impl of course can't be
// selected, and as there are hundreds of similar impls,
// considering them would significantly harm performance.

// This depends on the set of all impls for the trait. That is
// unfortunate. When we get red-green recompilation, we would like
// to have a way of knowing whether the set of relevant impls
// changed. The most naive
// way would be to compute the Vec of relevant impls and see whether
// it differs between compilations. That shouldn't be too slow by
// itself - we do quite a bit of work for each relevant impl anyway.
//
// If we want to be faster, we could have separate queries for
// blanket and non-blanket impls, and compare them separately.
// Note that we're using `SimplifyParams::Yes` to query `non_blanket_impls` while using
// `SimplifyParams::No` while actually adding them.
//
// I think we'll cross that bridge when we get to it.
if let Some(simp) = fast_reject::simplify_type(self, self_ty, true) {
// 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, SimplifyParams::Yes, StripReferences::No)
{
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 @@ -238,7 +227,9 @@ pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> Trait
continue;
}

if let Some(simplified_self_ty) = fast_reject::simplify_type(tcx, impl_self_ty, false) {
if let Some(simplified_self_ty) =
fast_reject::simplify_type(tcx, impl_self_ty, SimplifyParams::No, StripReferences::No)
{
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
} else {
impls.blanket_impls.push(impl_def_id);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ use crate::traits::{
self, Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext,
};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, fast_reject, Ty, TyCtxt};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::symbol::sym;
use rustc_span::DUMMY_SP;
use std::iter;
Expand Down Expand Up @@ -82,12 +83,11 @@ where
impl2_ref.iter().flat_map(|tref| tref.substs.types()),
)
.any(|(ty1, ty2)| {
let t1 = fast_reject::simplify_type(tcx, ty1, false);
let t2 = fast_reject::simplify_type(tcx, ty2, false);
let t1 = fast_reject::simplify_type(tcx, ty1, SimplifyParams::No, StripReferences::No);
let t2 = fast_reject::simplify_type(tcx, ty2, SimplifyParams::No, StripReferences::No);
if let (Some(t1), Some(t2)) = (t1, t2) {
// Simplified successfully
// Types cannot unify if they differ in their reference mutability or simplify to different types
t1 != t2 || ty1.ref_mutability() != ty2.ref_mutability()
t1 != t2
lcnr marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Types might unify
false
Expand Down
26 changes: 22 additions & 4 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ use rustc_hir::Item;
use rustc_hir::Node;
use rustc_middle::thir::abstract_const::NotConstEvaluatable;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fold::TypeFolder;
use rustc_middle::ty::{
self, fast_reject, AdtKind, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt,
TypeFoldable,
self, AdtKind, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable,
};
use rustc_session::DiagnosticMessageId;
use rustc_span::symbol::{kw, sym};
Expand Down Expand Up @@ -1439,14 +1439,32 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
) -> Vec<ty::TraitRef<'tcx>> {
let simp = fast_reject::simplify_type(self.tcx, trait_ref.skip_binder().self_ty(), true);
// We simplify params and strip references here.
//
// This both removes a lot of unhelpful suggestions, e.g.
// when searching for `&Foo: Trait` it doesn't suggestion `impl Trait for &Bar`,
// while also suggesting impls for `&Foo` when we're looking for `Foo: Trait`.
//
// The second thing isn't necessarily always a good thing, but
// any other simple setup results in a far worse output, so 🤷
let simp = fast_reject::simplify_type(
self.tcx,
trait_ref.skip_binder().self_ty(),
SimplifyParams::Yes,
StripReferences::Yes,
);
let all_impls = self.tcx.all_impls(trait_ref.def_id());

match simp {
Some(simp) => all_impls
.filter_map(|def_id| {
let imp = self.tcx.impl_trait_ref(def_id).unwrap();
let imp_simp = fast_reject::simplify_type(self.tcx, imp.self_ty(), true);
let imp_simp = fast_reject::simplify_type(
self.tcx,
imp.self_ty(),
SimplifyParams::Yes,
StripReferences::Yes,
);
if let Some(imp_simp) = imp_simp {
if simp != imp_simp {
return None;
Expand Down
21 changes: 16 additions & 5 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::dep_graph::{DepKind, DepNodeIndex};
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::thir::abstract_const::NotConstEvaluatable;
use rustc_middle::ty::fast_reject;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::relate::TypeRelation;
use rustc_middle::ty::subst::{GenericArgKind, Subst, SubstsRef};
Expand Down Expand Up @@ -2089,10 +2089,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
|(obligation_arg, impl_arg)| {
match (obligation_arg.unpack(), impl_arg.unpack()) {
(GenericArgKind::Type(obligation_ty), GenericArgKind::Type(impl_ty)) => {
let simplified_obligation_ty =
fast_reject::simplify_type(self.tcx(), obligation_ty, true);
let simplified_impl_ty =
fast_reject::simplify_type(self.tcx(), impl_ty, false);
// Note, we simplify parameters for the obligation but not the
// impl so that we do not reject a blanket impl but do reject
// more concrete impls if we're searching for `T: Trait`.
let simplified_obligation_ty = fast_reject::simplify_type(
self.tcx(),
obligation_ty,
SimplifyParams::Yes,
StripReferences::No,
);
let simplified_impl_ty = fast_reject::simplify_type(
self.tcx(),
impl_ty,
SimplifyParams::No,
StripReferences::No,
);

simplified_obligation_ty.is_some()
&& simplified_impl_ty.is_some()
Expand Down
Loading