Skip to content

Commit 6777702

Browse files
Add two new diagnostics: one for mismatch in generic arguments count, and another for mismatch in their kind
Also known as E0747 and E0107. And by the way, rewrite how we lower generic arguments and deduplicate it between paths and method calls. The new version is taken almost straight from rustc. This commit also changes the binders of `generic_defaults()`, to only include the binders of the arguments up to (and not including) the current argument. This make it easier to handle it in the rewritten lowering of generic args. It's also how rustc does it.
1 parent 5bbf2ce commit 6777702

22 files changed

+1206
-390
lines changed

crates/hir-def/src/generics.rs

+1
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ impl GenericParamData {
154154

155155
impl_from!(TypeParamData, ConstParamData, LifetimeParamData for GenericParamData);
156156

157+
#[derive(Debug, Clone, Copy)]
157158
pub enum GenericParamDataRef<'a> {
158159
TypeParamData(&'a TypeParamData),
159160
ConstParamData(&'a ConstParamData),

crates/hir-def/src/hir/type_ref.rs

+2
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ pub enum ConstRef {
522522
Scalar(Box<LiteralConstRef>),
523523
Path(Name),
524524
Complex(AstId<ast::ConstArg>),
525+
Infer,
525526
}
526527

527528
impl ConstRef {
@@ -553,6 +554,7 @@ impl ConstRef {
553554
ConstRef::Scalar(s) => s.fmt(f),
554555
ConstRef::Path(n) => n.display(self.0, self.2).fmt(f),
555556
ConstRef::Complex(_) => f.write_str("{const}"),
557+
ConstRef::Infer => f.write_str("_"),
556558
}
557559
}
558560
}

crates/hir-def/src/path.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ use syntax::ast;
2020

2121
pub use hir_expand::mod_path::{ModPath, PathKind, path};
2222

23-
pub use lower::hir_segment_to_ast_segment;
23+
pub use lower::{
24+
hir_assoc_type_binding_to_ast, hir_generic_arg_to_ast, hir_segment_to_ast_segment,
25+
};
2426

2527
#[derive(Debug, Clone, PartialEq, Eq)]
2628
pub enum ImportAlias {

crates/hir-def/src/path/lower.rs

+33-3
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,8 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option<
129129
args: iter::once(self_type)
130130
.chain(it.args.iter().cloned())
131131
.collect(),
132-
133132
has_self_type: true,
134-
bindings: it.bindings.clone(),
135-
parenthesized: it.parenthesized,
133+
..it
136134
},
137135
None => GenericArgs {
138136
args: Box::new([self_type]),
@@ -271,6 +269,7 @@ pub(super) fn lower_generic_args(
271269
lower_ctx: &mut LowerCtx<'_>,
272270
node: ast::GenericArgList,
273271
) -> Option<GenericArgs> {
272+
// This needs to be kept in sync with `hir_generic_arg_to_ast()`.
274273
let mut args = Vec::new();
275274
let mut bindings = Vec::new();
276275
for generic_arg in node.generic_args() {
@@ -281,6 +280,7 @@ pub(super) fn lower_generic_args(
281280
args.push(GenericArg::Type(type_ref));
282281
}
283282
ast::GenericArg::AssocTypeArg(assoc_type_arg) => {
283+
// This needs to be kept in sync with `hir_assoc_type_binding_to_ast()`.
284284
if assoc_type_arg.param_list().is_some() {
285285
// We currently ignore associated return type bounds.
286286
continue;
@@ -334,6 +334,36 @@ pub(super) fn lower_generic_args(
334334
})
335335
}
336336

337+
/// This function find the AST fragment that corresponds to an `AssociatedTypeBinding` in the HIR.
338+
pub fn hir_assoc_type_binding_to_ast(
339+
segment_args: &ast::GenericArgList,
340+
binding_idx: u32,
341+
) -> Option<ast::AssocTypeArg> {
342+
segment_args
343+
.generic_args()
344+
.filter_map(|arg| match arg {
345+
ast::GenericArg::AssocTypeArg(it) => Some(it),
346+
_ => None,
347+
})
348+
.filter(|binding| binding.param_list().is_none() && binding.name_ref().is_some())
349+
.nth(binding_idx as usize)
350+
}
351+
352+
/// This function find the AST generic argument from the one in the HIR. Does not support the `Self` argument.
353+
pub fn hir_generic_arg_to_ast(
354+
args: &ast::GenericArgList,
355+
arg_idx: u32,
356+
has_self_arg: bool,
357+
) -> Option<ast::GenericArg> {
358+
args.generic_args()
359+
.filter(|arg| match arg {
360+
ast::GenericArg::AssocTypeArg(_) => false,
361+
ast::GenericArg::LifetimeArg(arg) => arg.lifetime().is_some(),
362+
ast::GenericArg::ConstArg(_) | ast::GenericArg::TypeArg(_) => true,
363+
})
364+
.nth(arg_idx as usize - has_self_arg as usize)
365+
}
366+
337367
/// Collect `GenericArgs` from the parts of a fn-like path, i.e. `Fn(X, Y)
338368
/// -> Z` (which desugars to `Fn<(X, Y), Output=Z>`).
339369
fn lower_generic_args_from_fn_path(

crates/hir-ty/src/builder.rs

+19-20
Original file line numberDiff line numberDiff line change
@@ -305,29 +305,28 @@ impl TyBuilder<hir_def::AdtId> {
305305
// Note that we're building ADT, so we never have parent generic parameters.
306306
let defaults = db.generic_defaults(self.data.into());
307307

308-
for default_ty in &defaults[self.vec.len()..] {
309-
// NOTE(skip_binders): we only check if the arg type is error type.
310-
if let Some(x) = default_ty.skip_binders().ty(Interner) {
311-
if x.is_unknown() {
312-
self.vec.push(fallback().cast(Interner));
313-
continue;
308+
if let Some(defaults) = defaults.get(self.vec.len()..) {
309+
for default_ty in defaults {
310+
// NOTE(skip_binders): we only check if the arg type is error type.
311+
if let Some(x) = default_ty.skip_binders().ty(Interner) {
312+
if x.is_unknown() {
313+
self.vec.push(fallback().cast(Interner));
314+
continue;
315+
}
314316
}
317+
// Each default can only depend on the previous parameters.
318+
self.vec.push(default_ty.clone().substitute(Interner, &*self.vec).cast(Interner));
315319
}
316-
// Each default can only depend on the previous parameters.
317-
let subst_so_far = Substitution::from_iter(
318-
Interner,
319-
self.vec
320-
.iter()
321-
.cloned()
322-
.chain(self.param_kinds[self.vec.len()..].iter().map(|it| match it {
323-
ParamKind::Type => TyKind::Error.intern(Interner).cast(Interner),
324-
ParamKind::Lifetime => error_lifetime().cast(Interner),
325-
ParamKind::Const(ty) => unknown_const_as_generic(ty.clone()),
326-
}))
327-
.take(self.param_kinds.len()),
328-
);
329-
self.vec.push(default_ty.clone().substitute(Interner, &subst_so_far).cast(Interner));
330320
}
321+
322+
// The defaults may be missing if no param has default, so fill that.
323+
let filler = self.param_kinds[self.vec.len()..].iter().map(|x| match x {
324+
ParamKind::Type => fallback().cast(Interner),
325+
ParamKind::Const(ty) => unknown_const_as_generic(ty.clone()),
326+
ParamKind::Lifetime => error_lifetime().cast(Interner),
327+
});
328+
self.vec.extend(filler.casted(Interner));
329+
331330
self
332331
}
333332

crates/hir-ty/src/db.rs

+3
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,9 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> + std::fmt::Debug {
193193
def: GenericDefId,
194194
) -> (GenericDefaults, Diagnostics);
195195

196+
/// This returns an empty list if no parameter has default.
197+
///
198+
/// The binders of the returned defaults are only up to (not including) this parameter.
196199
#[salsa::invoke_actual(crate::lower::generic_defaults_query)]
197200
fn generic_defaults(&self, def: GenericDefId) -> GenericDefaults;
198201

crates/hir-ty/src/display.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1632,7 +1632,7 @@ fn generic_args_sans_defaults<'ga>(
16321632
Some(default_parameter) => {
16331633
// !is_err(default_parameter.skip_binders())
16341634
// &&
1635-
arg != &default_parameter.clone().substitute(Interner, &parameters)
1635+
arg != &default_parameter.clone().substitute(Interner, &parameters[..i])
16361636
}
16371637
}
16381638
};

crates/hir-ty/src/generics.rs

+5-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ use hir_def::{
2121
type_ref::TypesMap,
2222
};
2323
use itertools::chain;
24-
use stdx::TupleExt;
2524
use triomphe::Arc;
2625

2726
use crate::{Interner, Substitution, db::HirDatabase, lt_to_placeholder_idx, to_placeholder_idx};
@@ -77,12 +76,6 @@ impl Generics {
7776
self.params.iter_type_or_consts()
7877
}
7978

80-
pub(crate) fn iter_self_type_or_consts_id(
81-
&self,
82-
) -> impl DoubleEndedIterator<Item = GenericParamId> + '_ {
83-
self.params.iter_type_or_consts().map(from_toc_id(self)).map(TupleExt::head)
84-
}
85-
8679
/// Iterate over the params followed by the parent params.
8780
pub(crate) fn iter(
8881
&self,
@@ -132,6 +125,10 @@ impl Generics {
132125
self.params.len()
133126
}
134127

128+
pub(crate) fn len_lifetimes_self(&self) -> usize {
129+
self.params.len_lifetimes()
130+
}
131+
135132
/// (parent total, self param, type params, const params, impl trait list, lifetimes)
136133
pub(crate) fn provenance_split(&self) -> (usize, bool, usize, usize, usize, usize) {
137134
let mut self_param = false;
@@ -147,7 +144,7 @@ impl Generics {
147144
TypeOrConstParamData::ConstParamData(_) => const_params += 1,
148145
});
149146

150-
let lifetime_params = self.params.iter_lt().count();
147+
let lifetime_params = self.params.len_lifetimes();
151148

152149
let parent_len = self.parent_generics().map_or(0, Generics::len);
153150
(parent_len, self_param, type_params, const_params, impl_trait_params, lifetime_params)

crates/hir-ty/src/infer.rs

+25-8
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ use chalk_ir::{
3434
};
3535
use either::Either;
3636
use hir_def::{
37-
AdtId, AssocItemId, DefWithBodyId, FieldId, FunctionId, ImplId, ItemContainerId, Lookup,
38-
TraitId, TupleFieldId, TupleId, TypeAliasId, VariantId,
37+
AdtId, AssocItemId, DefWithBodyId, FieldId, FunctionId, GenericDefId, GenericParamId, ImplId,
38+
ItemContainerId, Lookup, TraitId, TupleFieldId, TupleId, TypeAliasId, VariantId,
3939
builtin_type::{BuiltinInt, BuiltinType, BuiltinUint},
4040
data::{ConstData, StaticData},
4141
expr_store::{Body, HygieneId},
@@ -56,8 +56,9 @@ use triomphe::Arc;
5656

5757
use crate::{
5858
AliasEq, AliasTy, Binders, ClosureId, Const, DomainGoal, GenericArg, Goal, ImplTraitId,
59-
ImplTraitIdx, InEnvironment, Interner, Lifetime, OpaqueTyId, ParamLoweringMode,
60-
PathLoweringDiagnostic, ProjectionTy, Substitution, TraitEnvironment, Ty, TyBuilder, TyExt,
59+
ImplTraitIdx, InEnvironment, IncorrectGenericsLenKind, Interner, Lifetime, OpaqueTyId,
60+
ParamLoweringMode, PathLoweringDiagnostic, ProjectionTy, Substitution, TraitEnvironment, Ty,
61+
TyBuilder, TyExt,
6162
db::HirDatabase,
6263
fold_tys,
6364
generics::Generics,
@@ -67,7 +68,7 @@ use crate::{
6768
expr::ExprIsRead,
6869
unify::InferenceTable,
6970
},
70-
lower::{ImplTraitLoweringMode, diagnostics::TyLoweringDiagnostic},
71+
lower::{GenericArgsPosition, ImplTraitLoweringMode, diagnostics::TyLoweringDiagnostic},
7172
mir::MirSpan,
7273
to_assoc_type_id,
7374
traits::FnTrait,
@@ -286,6 +287,20 @@ pub enum InferenceDiagnostic {
286287
node: ExprOrPatId,
287288
diag: PathLoweringDiagnostic,
288289
},
290+
MethodCallIncorrectGenericsLen {
291+
expr: ExprId,
292+
provided_count: u32,
293+
expected_count: u32,
294+
kind: IncorrectGenericsLenKind,
295+
def: GenericDefId,
296+
},
297+
MethodCallIncorrectGenericsOrder {
298+
expr: ExprId,
299+
param_id: GenericParamId,
300+
arg_idx: u32,
301+
/// Whether the `GenericArgs` contains a `Self` arg.
302+
has_self_arg: bool,
303+
},
289304
}
290305

291306
/// A mismatch between an expected and an inferred type.
@@ -920,6 +935,7 @@ impl<'a> InferenceContext<'a> {
920935
self.with_ty_lowering(&data.types_map, InferenceTyDiagnosticSource::Signature, |ctx| {
921936
ctx.type_param_mode(ParamLoweringMode::Placeholder)
922937
.impl_trait_mode(ImplTraitLoweringMode::Param);
938+
ctx.in_fn_signature = true;
923939
data.params.iter().map(|&type_ref| ctx.lower_ty(type_ref)).collect::<Vec<_>>()
924940
});
925941
// Check if function contains a va_list, if it does then we append it to the parameter types
@@ -961,8 +977,9 @@ impl<'a> InferenceContext<'a> {
961977
let return_ty =
962978
self.with_ty_lowering(&data.types_map, InferenceTyDiagnosticSource::Signature, |ctx| {
963979
ctx.type_param_mode(ParamLoweringMode::Placeholder)
964-
.impl_trait_mode(ImplTraitLoweringMode::Opaque)
965-
.lower_ty(return_ty)
980+
.impl_trait_mode(ImplTraitLoweringMode::Opaque);
981+
ctx.in_fn_signature = true;
982+
ctx.lower_ty(return_ty)
966983
});
967984
let return_ty = self.insert_type_vars(return_ty);
968985

@@ -1499,7 +1516,7 @@ impl<'a> InferenceContext<'a> {
14991516
&self.diagnostics,
15001517
InferenceTyDiagnosticSource::Body,
15011518
);
1502-
let mut path_ctx = ctx.at_path(path, node);
1519+
let mut path_ctx = ctx.at_path(path, node, GenericArgsPosition::Value);
15031520
let (resolution, unresolved) = if value_ns {
15041521
let Some(res) = path_ctx.resolve_path_in_value_ns(HygieneId::ROOT) else {
15051522
return (self.err_ty(), None);

crates/hir-ty/src/infer/diagnostics.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use either::Either;
99
use hir_def::{TypeOwnerId, hir::ExprOrPatId, path::Path, resolver::Resolver, type_ref::TypesMap};
1010
use la_arena::{Idx, RawIdx};
1111

12+
use crate::lower::GenericArgsPosition;
1213
use crate::{
1314
InferenceDiagnostic, InferenceTyDiagnosticSource, TyLoweringContext, TyLoweringDiagnostic,
1415
db::HirDatabase,
@@ -71,6 +72,7 @@ impl<'a> InferenceTyLoweringContext<'a> {
7172
&'b mut self,
7273
path: &'b Path,
7374
node: ExprOrPatId,
75+
position: GenericArgsPosition,
7476
) -> PathLoweringContext<'b, 'a> {
7577
let on_diagnostic = PathDiagnosticCallback {
7678
data: Either::Right(PathDiagnosticCallbackData { diagnostics: self.diagnostics, node }),
@@ -80,13 +82,14 @@ impl<'a> InferenceTyLoweringContext<'a> {
8082
.push(InferenceDiagnostic::PathDiagnostic { node: data.node, diag });
8183
},
8284
};
83-
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path)
85+
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path, position)
8486
}
8587

8688
#[inline]
8789
pub(super) fn at_path_forget_diagnostics<'b>(
8890
&'b mut self,
8991
path: &'b Path,
92+
position: GenericArgsPosition,
9093
) -> PathLoweringContext<'b, 'a> {
9194
let on_diagnostic = PathDiagnosticCallback {
9295
data: Either::Right(PathDiagnosticCallbackData {
@@ -95,7 +98,7 @@ impl<'a> InferenceTyLoweringContext<'a> {
9598
}),
9699
callback: |_data, _, _diag| {},
97100
};
98-
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path)
101+
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path, position)
99102
}
100103

101104
#[inline]

0 commit comments

Comments
 (0)