Skip to content

fix: Fix lifetime parameters moving parameter defaults #17529

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.

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

Merged
merged 1 commit into from
Jul 2, 2024
Merged
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions crates/hir-ty/src/builder.rs
Original file line number Diff line number Diff line change
@@ -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);
}
@@ -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() {
13 changes: 12 additions & 1 deletion crates/hir-ty/src/generics.rs
Original file line number Diff line number Diff line change
@@ -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};

@@ -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)
}

@@ -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,
129 changes: 66 additions & 63 deletions crates/hir-ty/src/lower.rs
Original file line number Diff line number Diff line change
@@ -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,
@@ -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,
@@ -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,
@@ -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)
}

23 changes: 23 additions & 0 deletions crates/hir-ty/src/tests/regression.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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))
}