Skip to content

Commit

Permalink
Merge #11743
Browse files Browse the repository at this point in the history
11743: fix: Properly unify receivers during method resolution r=flodiebold a=flodiebold

This fixes our type inference problems with `DynMap`; the problem there were the projection types in
```rust
impl<P: Policy> KeyMap<Key<P::K, P::V, P>>
```
which messed up the naive type equality check we did. It also actually simplifies the logic though, IMO. I also added a fix for associated const resolution that I noticed (visibility not being taken into account).

Co-authored-by: Florian Diebold <[email protected]>
  • Loading branch information
bors[bot] and flodiebold authored Mar 17, 2022
2 parents 631b504 + 60aeb8f commit dbc697c
Show file tree
Hide file tree
Showing 11 changed files with 295 additions and 276 deletions.
8 changes: 2 additions & 6 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1541,9 +1541,7 @@ impl SelfParam {

impl HasVisibility for Function {
fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
let function_data = db.function_data(self.id);
let visibility = &function_data.visibility;
visibility.resolve(db.upcast(), &self.id.resolver(db.upcast()))
db.function_visibility(self.id)
}
}

Expand Down Expand Up @@ -1594,9 +1592,7 @@ impl Const {

impl HasVisibility for Const {
fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
let function_data = db.const_data(self.id);
let visibility = &function_data.visibility;
visibility.resolve(db.upcast(), &self.id.resolver(db.upcast()))
db.const_visibility(self.id)
}
}

Expand Down
4 changes: 4 additions & 0 deletions crates/hir_def/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,13 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast<dyn AstDatabase> {
#[salsa::invoke(visibility::field_visibilities_query)]
fn field_visibilities(&self, var: VariantId) -> Arc<ArenaMap<LocalFieldId, Visibility>>;

// FIXME: unify function_visibility and const_visibility?
#[salsa::invoke(visibility::function_visibility_query)]
fn function_visibility(&self, def: FunctionId) -> Visibility;

#[salsa::invoke(visibility::const_visibility_query)]
fn const_visibility(&self, def: ConstId) -> Visibility;

#[salsa::transparent]
fn crate_limits(&self, crate_id: CrateId) -> CrateLimits;
}
Expand Down
8 changes: 7 additions & 1 deletion crates/hir_def/src/visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
nameres::DefMap,
path::{ModPath, PathKind},
resolver::HasResolver,
FunctionId, HasModule, LocalFieldId, ModuleId, VariantId,
ConstId, FunctionId, HasModule, LocalFieldId, ModuleId, VariantId,
};

/// Visibility of an item, not yet resolved.
Expand Down Expand Up @@ -234,3 +234,9 @@ pub(crate) fn function_visibility_query(db: &dyn DefDatabase, def: FunctionId) -
let resolver = def.resolver(db);
db.function_data(def).visibility.resolve(db, &resolver)
}

/// Resolve visibility of a const.
pub(crate) fn const_visibility_query(db: &dyn DefDatabase, def: ConstId) -> Visibility {
let resolver = def.resolver(db);
db.const_data(def).visibility.resolve(db, &resolver)
}
17 changes: 13 additions & 4 deletions crates/hir_ty/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ use hir_def::{
use smallvec::SmallVec;

use crate::{
consteval::unknown_const_as_generic, db::HirDatabase, primitive, to_assoc_type_id,
to_chalk_trait_id, utils::generics, Binders, CallableSig, ConstData, ConstValue, GenericArg,
GenericArgData, Interner, ProjectionTy, Substitution, TraitRef, Ty, TyDefId, TyExt, TyKind,
ValueTyDefId,
consteval::unknown_const_as_generic, db::HirDatabase, infer::unify::InferenceTable, primitive,
to_assoc_type_id, to_chalk_trait_id, utils::generics, Binders, CallableSig, ConstData,
ConstValue, GenericArg, GenericArgData, Interner, ProjectionTy, Substitution, TraitRef, Ty,
TyDefId, TyExt, TyKind, ValueTyDefId,
};

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -111,6 +111,15 @@ impl<D> TyBuilder<D> {
this
}

pub(crate) fn fill_with_inference_vars(self, table: &mut InferenceTable) -> Self {
self.fill(|x| match x {
ParamKind::Type => GenericArgData::Ty(table.new_type_var()).intern(Interner),
ParamKind::Const(ty) => {
GenericArgData::Const(table.new_const_var(ty.clone())).intern(Interner)
}
})
}

pub fn fill(mut self, filler: impl FnMut(&ParamKind) -> GenericArg) -> Self {
self.vec.extend(self.param_kinds.iter().skip(self.vec.len()).map(filler));
assert_eq!(self.remaining(), 0);
Expand Down
18 changes: 4 additions & 14 deletions crates/hir_ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,16 @@ use rustc_hash::FxHashMap;
use stdx::impl_from;

use crate::{
builder::ParamKind, db::HirDatabase, fold_tys_and_consts, infer::coerce::CoerceMany,
lower::ImplTraitLoweringMode, to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal,
GenericArg, GenericArgData, Goal, InEnvironment, Interner, ProjectionTy, Substitution,
TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind,
db::HirDatabase, fold_tys_and_consts, infer::coerce::CoerceMany, lower::ImplTraitLoweringMode,
to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal, GenericArg, Goal, InEnvironment,
Interner, ProjectionTy, Substitution, TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind,
};

// This lint has a false positive here. See the link below for details.
//
// https://github.com/rust-lang/rust/issues/57411
#[allow(unreachable_pub)]
pub use unify::could_unify;
pub(crate) use unify::unify;

pub(crate) mod unify;
mod path;
Expand Down Expand Up @@ -657,15 +655,7 @@ impl<'a> InferenceContext<'a> {
}
TypeNs::TypeAliasId(it) => {
let ty = TyBuilder::def_ty(self.db, it.into())
.fill(|x| match x {
ParamKind::Type => {
GenericArgData::Ty(self.table.new_type_var()).intern(Interner)
}
ParamKind::Const(ty) => {
GenericArgData::Const(self.table.new_const_var(ty.clone()))
.intern(Interner)
}
})
.fill_with_inference_vars(&mut self.table)
.build();
self.resolve_variant_on_alias(ty, unresolved, path)
}
Expand Down
24 changes: 3 additions & 21 deletions crates/hir_ty/src/infer/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@ impl<'a> InferenceContext<'a> {
ParamKind::Type => {
GenericArgData::Ty(TyKind::Error.intern(Interner)).intern(Interner)
}
ParamKind::Const(_) => {
GenericArgData::Const(consteval::usize_const(None)).intern(Interner)
}
ParamKind::Const(ty) => consteval::unknown_const_as_generic(ty.clone()),
})
})
.build();
Expand Down Expand Up @@ -249,15 +247,7 @@ impl<'a> InferenceContext<'a> {
let substs = match container {
ItemContainerId::ImplId(impl_id) => {
let impl_substs = TyBuilder::subst_for_def(self.db, impl_id)
.fill(|x| match x {
ParamKind::Type => {
GenericArgData::Ty(self.table.new_type_var()).intern(Interner)
}
ParamKind::Const(ty) => {
GenericArgData::Const(self.table.new_const_var(ty.clone()))
.intern(Interner)
}
})
.fill_with_inference_vars(&mut self.table)
.build();
let impl_self_ty =
self.db.impl_self_ty(impl_id).substitute(Interner, &impl_substs);
Expand All @@ -268,15 +258,7 @@ impl<'a> InferenceContext<'a> {
// we're picking this method
let trait_ref = TyBuilder::trait_ref(self.db, trait_)
.push(ty.clone())
.fill(|x| match x {
ParamKind::Type => {
GenericArgData::Ty(self.table.new_type_var()).intern(Interner)
}
ParamKind::Const(ty) => {
GenericArgData::Const(self.table.new_const_var(ty.clone()))
.intern(Interner)
}
})
.fill_with_inference_vars(&mut self.table)
.build();
self.push_obligation(trait_ref.clone().cast(Interner));
Some(trait_ref.substitution)
Expand Down
17 changes: 15 additions & 2 deletions crates/hir_ty/src/infer/unify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub(crate) struct InferenceTable<'a> {

pub(crate) struct InferenceTableSnapshot {
var_table_snapshot: chalk_solve::infer::InferenceSnapshot<Interner>,
// FIXME: snapshot pending_obligations?
pending_obligations: Vec<Canonicalized<InEnvironment<Goal>>>,
type_variable_table_snapshot: Vec<TypeVariableData>,
}

Expand Down Expand Up @@ -365,12 +365,25 @@ impl<'a> InferenceTable<'a> {
pub(crate) fn snapshot(&mut self) -> InferenceTableSnapshot {
let var_table_snapshot = self.var_unification_table.snapshot();
let type_variable_table_snapshot = self.type_variable_table.clone();
InferenceTableSnapshot { var_table_snapshot, type_variable_table_snapshot }
let pending_obligations = self.pending_obligations.clone();
InferenceTableSnapshot {
var_table_snapshot,
pending_obligations,
type_variable_table_snapshot,
}
}

pub(crate) fn rollback_to(&mut self, snapshot: InferenceTableSnapshot) {
self.var_unification_table.rollback_to(snapshot.var_table_snapshot);
self.type_variable_table = snapshot.type_variable_table_snapshot;
self.pending_obligations = snapshot.pending_obligations;
}

pub(crate) fn run_in_snapshot<T>(&mut self, f: impl FnOnce(&mut InferenceTable) -> T) -> T {
let snapshot = self.snapshot();
let result = f(self);
self.rollback_to(snapshot);
result
}

/// Checks an obligation without registering it. Useful mostly to check
Expand Down
28 changes: 27 additions & 1 deletion crates/hir_ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use std::cell::{Cell, RefCell};
use std::{iter, sync::Arc};

use base_db::CrateId;
use chalk_ir::fold::Fold;
use chalk_ir::interner::HasInterner;
use chalk_ir::{cast::Cast, fold::Shift, Mutability, Safety};
use hir_def::generics::TypeOrConstParamData;
use hir_def::intern::Interned;
Expand Down Expand Up @@ -36,7 +38,6 @@ use stdx::{impl_from, never};
use syntax::{ast, SmolStr};

use crate::consteval::{path_to_const, unknown_const_as_generic, unknown_const_usize, usize_const};
use crate::method_resolution::fallback_bound_vars;
use crate::utils::Generics;
use crate::{all_super_traits, make_binders, Const, GenericArgData, ParamKind};
use crate::{
Expand Down Expand Up @@ -1701,3 +1702,28 @@ pub(crate) fn const_or_path_to_chalk(
}
}
}

/// This replaces any 'free' Bound vars in `s` (i.e. those with indices past
/// num_vars_to_keep) by `TyKind::Unknown`.
fn fallback_bound_vars<T: Fold<Interner> + HasInterner<Interner = Interner>>(
s: T,
num_vars_to_keep: usize,
) -> T::Result {
crate::fold_free_vars(
s,
|bound, binders| {
if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST {
TyKind::Error.intern(Interner)
} else {
bound.shifted_in_from(binders).to_ty(Interner)
}
},
|ty, bound, binders| {
if bound.index >= num_vars_to_keep && bound.debruijn == DebruijnIndex::INNERMOST {
consteval::unknown_const(ty.clone())
} else {
bound.shifted_in_from(binders).to_const(Interner, ty)
}
},
)
}
Loading

0 comments on commit dbc697c

Please sign in to comment.