Skip to content

Commit

Permalink
Fix lifetime parameters moving paramter defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Jul 2, 2024
1 parent 1b283db commit 1a929d6
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 68 deletions.
12 changes: 10 additions & 2 deletions crates/hir-ty/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,14 @@ impl<D> TyBuilder<D> {
}

fn build_internal(self) -> (D, Substitution) {
assert_eq!(self.vec.len(), self.param_kinds.len(), "{:?}", &self.param_kinds);
assert_eq!(
self.vec.len(),
self.param_kinds.len(),
"{} args received, {} expected ({:?})",
self.vec.len(),
self.param_kinds.len(),
&self.param_kinds
);
for (a, e) in self.vec.iter().zip(self.param_kinds.iter()) {
self.assert_match_kind(a, e);
}
Expand Down Expand Up @@ -297,7 +304,8 @@ impl TyBuilder<hir_def::AdtId> {
) -> Self {
// Note that we're building ADT, so we never have parent generic parameters.
let defaults = db.generic_defaults(self.data.into());
for default_ty in defaults.iter().skip(self.vec.len()) {

for default_ty in &defaults[self.vec.len()..] {
// NOTE(skip_binders): we only check if the arg type is error type.
if let Some(x) = default_ty.skip_binders().ty(Interner) {
if x.is_unknown() {
Expand Down
13 changes: 12 additions & 1 deletion crates/hir-ty/src/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use hir_def::{
LocalLifetimeParamId, LocalTypeOrConstParamId, Lookup, TypeOrConstParamId, TypeParamId,
};
use intern::Interned;
use stdx::TupleExt;

use crate::{db::HirDatabase, lt_to_placeholder_idx, to_placeholder_idx, Interner, Substitution};

Expand Down Expand Up @@ -57,7 +58,7 @@ impl Generics {
self.iter_self().map(|(id, _)| id)
}

fn iter_parent_id(&self) -> impl Iterator<Item = GenericParamId> + '_ {
pub(crate) fn iter_parent_id(&self) -> impl Iterator<Item = GenericParamId> + '_ {
self.iter_parent().map(|(id, _)| id)
}

Expand All @@ -67,6 +68,16 @@ impl Generics {
self.params.iter_type_or_consts()
}

pub(crate) fn iter_self_type_or_consts_id(
&self,
) -> impl DoubleEndedIterator<Item = GenericParamId> + '_ {
self.params.iter_type_or_consts().map(from_toc_id(self)).map(TupleExt::head)
}

pub(crate) fn iter_self_lt_id(&self) -> impl DoubleEndedIterator<Item = GenericParamId> + '_ {
self.params.iter_lt().map(from_lt_id(self)).map(TupleExt::head)
}

/// Iterate over the params followed by the parent params.
pub(crate) fn iter(
&self,
Expand Down
129 changes: 66 additions & 63 deletions crates/hir-ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,13 +812,13 @@ impl<'a> TyLoweringContext<'a> {
infer_args: bool,
explicit_self_ty: Option<Ty>,
) -> Substitution {
// Remember that the item's own generic args come before its parent's.
let mut substs = Vec::new();
let def = if let Some(d) = def {
d
} else {
return Substitution::empty(Interner);
};
let Some(def) = def else { return Substitution::empty(Interner) };

// Order is
// - Optional Self parameter
// - Type or Const parameters
// - Lifetime parameters
// - Parent parameters
let def_generics = generics(self.db.upcast(), def);
let (
parent_params,
Expand All @@ -832,45 +832,46 @@ impl<'a> TyLoweringContext<'a> {
self_param as usize + type_params + const_params + impl_trait_params + lifetime_params;
let total_len = parent_params + item_len;

let ty_error = TyKind::Error.intern(Interner).cast(Interner);
let mut substs = Vec::new();

let mut def_generic_iter = def_generics.iter_id();
// we need to iterate the lifetime and type/const params separately as our order of them
// differs from the supplied syntax

let fill_self_params = || {
let ty_error = || TyKind::Error.intern(Interner).cast(Interner);
let mut def_toc_iter = def_generics.iter_self_type_or_consts_id();
let mut def_lt_iter = def_generics.iter_self_lt_id();
let fill_self_param = || {
if self_param {
let self_ty =
explicit_self_ty.map(|x| x.cast(Interner)).unwrap_or_else(|| ty_error.clone());
let self_ty = explicit_self_ty.map(|x| x.cast(Interner)).unwrap_or_else(ty_error);

if let Some(id) = def_generic_iter.next() {
assert!(matches!(
id,
GenericParamId::TypeParamId(_) | GenericParamId::LifetimeParamId(_)
));
if let Some(id) = def_toc_iter.next() {
assert!(matches!(id, GenericParamId::TypeParamId(_)));
substs.push(self_ty);
}
}
};
let mut had_explicit_args = false;

if let Some(generic_args) = &args_and_bindings {
if !generic_args.has_self_type {
fill_self_params();
let mut lifetimes = SmallVec::<[_; 1]>::new();
if let Some(&GenericArgs { ref args, has_self_type, .. }) = args_and_bindings {
if !has_self_type {
fill_self_param();
}
let expected_num = if generic_args.has_self_type {
let expected_num = if has_self_type {
self_param as usize + type_params + const_params
} else {
type_params + const_params
};
let skip = if generic_args.has_self_type && !self_param { 1 } else { 0 };
// if args are provided, it should be all of them, but we can't rely on that
for arg in generic_args
.args
let skip = if has_self_type && !self_param { 1 } else { 0 };
// if non-lifetime args are provided, it should be all of them, but we can't rely on that
for arg in args
.iter()
.filter(|arg| !matches!(arg, GenericArg::Lifetime(_)))
.skip(skip)
.take(expected_num)
{
if let Some(id) = def_generic_iter.next() {
if let Some(id) = def_toc_iter.next() {
had_explicit_args = true;
let arg = generic_arg_to_chalk(
self.db,
id,
Expand All @@ -880,20 +881,16 @@ impl<'a> TyLoweringContext<'a> {
|_, const_ref, ty| self.lower_const(const_ref, ty),
|_, lifetime_ref| self.lower_lifetime(lifetime_ref),
);
had_explicit_args = true;
substs.push(arg);
}
}

for arg in generic_args
.args
for arg in args
.iter()
.filter(|arg| matches!(arg, GenericArg::Lifetime(_)))
.take(lifetime_params)
{
// Taking into the fact that def_generic_iter will always have lifetimes at the end
// Should have some test cases tho to test this behaviour more properly
if let Some(id) = def_generic_iter.next() {
if let Some(id) = def_lt_iter.next() {
let arg = generic_arg_to_chalk(
self.db,
id,
Expand All @@ -903,59 +900,65 @@ impl<'a> TyLoweringContext<'a> {
|_, const_ref, ty| self.lower_const(const_ref, ty),
|_, lifetime_ref| self.lower_lifetime(lifetime_ref),
);
had_explicit_args = true;
substs.push(arg);
lifetimes.push(arg);
}
}
} else {
fill_self_params();
fill_self_param();
}

// These params include those of parent.
let remaining_params: SmallVec<[_; 2]> = def_generic_iter
.map(|id| match id {
GenericParamId::ConstParamId(x) => {
unknown_const_as_generic(self.db.const_param_ty(x))
}
GenericParamId::TypeParamId(_) => ty_error.clone(),
GenericParamId::LifetimeParamId(_) => error_lifetime().cast(Interner),
})
.collect();
assert_eq!(remaining_params.len() + substs.len(), total_len);

let param_to_err = |id| match id {
GenericParamId::ConstParamId(x) => unknown_const_as_generic(self.db.const_param_ty(x)),
GenericParamId::TypeParamId(_) => ty_error(),
GenericParamId::LifetimeParamId(_) => error_lifetime().cast(Interner),
};
// handle defaults. In expression or pattern path segments without
// explicitly specified type arguments, missing type arguments are inferred
// (i.e. defaults aren't used).
// Generic parameters for associated types are not supposed to have defaults, so we just
// ignore them.
let is_assoc_ty = if let GenericDefId::TypeAliasId(id) = def {
let container = id.lookup(self.db.upcast()).container;
matches!(container, ItemContainerId::TraitId(_))
} else {
false
let is_assoc_ty = || match def {
GenericDefId::TypeAliasId(id) => {
matches!(id.lookup(self.db.upcast()).container, ItemContainerId::TraitId(_))
}
_ => false,
};
if !is_assoc_ty && (!infer_args || had_explicit_args) {
let defaults = self.db.generic_defaults(def);
assert_eq!(total_len, defaults.len());
if (!infer_args || had_explicit_args) && !is_assoc_ty() {
let defaults = &*self.db.generic_defaults(def);
let (item, _parent) = defaults.split_at(item_len);
let (toc, lt) = item.split_at(item_len - lifetime_params);
let parent_from = item_len - substs.len();

for (idx, default_ty) in defaults[substs.len()..item_len].iter().enumerate() {
let mut rem =
def_generics.iter_id().skip(substs.len()).map(param_to_err).collect::<Vec<_>>();
// Fill in defaults for type/const params
for (idx, default_ty) in toc[substs.len()..].iter().enumerate() {
// each default can depend on the previous parameters
let substs_so_far = Substitution::from_iter(
Interner,
substs.iter().cloned().chain(remaining_params[idx..].iter().cloned()),
substs.iter().cloned().chain(rem[idx..].iter().cloned()),
);
substs.push(default_ty.clone().substitute(Interner, &substs_so_far));
}

// Keep parent's params as unknown.
let mut remaining_params = remaining_params;
substs.extend(remaining_params.drain(parent_from..));
let n_lifetimes = lifetimes.len();
// Fill in deferred lifetimes
substs.extend(lifetimes);
// Fill in defaults for lifetime params
for default_ty in &lt[n_lifetimes..] {
// these are always errors so skipping is fine
substs.push(default_ty.skip_binders().clone());
}
// Fill in remaining def params and parent params
substs.extend(rem.drain(parent_from..));
} else {
substs.extend(remaining_params);
substs.extend(def_toc_iter.map(param_to_err));
// Fill in deferred lifetimes
substs.extend(lifetimes);
// Fill in remaining def params and parent params
substs.extend(def_generics.iter_id().skip(substs.len()).map(param_to_err));
}

assert_eq!(substs.len(), total_len);
assert_eq!(substs.len(), total_len, "expected {} substs, got {}", total_len, substs.len());
Substitution::from_iter(Interner, substs)
}

Expand Down
23 changes: 23 additions & 0 deletions crates/hir-ty/src/tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2018,3 +2018,26 @@ fn tait_async_stack_overflow_17199() {
"#,
);
}

#[test]
fn lifetime_params_move_param_defaults() {
check_types(
r#"
pub struct Thing<'s, T = u32>;
impl <'s> Thing<'s> {
pub fn new() -> Thing<'s> {
Thing
//^^^^^ Thing<'?, u32>
}
}
fn main() {
let scope =
//^^^^^ &'? Thing<'?, u32>
&Thing::new();
//^^^^^^^^^^^^ Thing<'?, u32>
}
"#,
);
}
4 changes: 2 additions & 2 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3602,9 +3602,9 @@ impl ConstParam {
}

fn generic_arg_from_param(db: &dyn HirDatabase, id: TypeOrConstParamId) -> Option<GenericArg> {
let params = db.generic_defaults(id.parent);
let local_idx = hir_ty::param_idx(db, id)?;
let ty = params.get(local_idx)?.clone();
let defaults = db.generic_defaults(id.parent);
let ty = defaults.get(local_idx)?.clone();
let subst = TyBuilder::placeholder_subst(db, id.parent);
Some(ty.substitute(Interner, &subst))
}
Expand Down

0 comments on commit 1a929d6

Please sign in to comment.