From 2be7e26d7dfad29756f6ff57e0ff7a8ce52171da Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 27 Mar 2022 19:10:31 +0200 Subject: [PATCH 1/2] Move mismatched-arg-count diagnostic to inference --- crates/hir/src/lib.rs | 21 +++++---- crates/hir_ty/src/diagnostics/expr.rs | 66 ++------------------------- crates/hir_ty/src/infer.rs | 1 + crates/hir_ty/src/infer/expr.rs | 62 +++++++++++++++++++------ 4 files changed, 67 insertions(+), 83 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 18de04b16d..c07c017c3d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1183,6 +1183,19 @@ impl DefWithBody { .expect("break outside of loop in synthetic syntax"); acc.push(BreakOutsideOfLoop { expr }.into()) } + hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => { + match source_map.expr_syntax(*call_expr) { + Ok(source_ptr) => acc.push( + MismatchedArgCount { + call_expr: source_ptr, + expected: *expected, + found: *found, + } + .into(), + ), + Err(SyntheticSyntax) => (), + } + } } } for (expr, mismatch) in infer.expr_type_mismatches() { @@ -1297,14 +1310,6 @@ impl DefWithBody { ); } } - BodyValidationDiagnostic::MismatchedArgCount { call_expr, expected, found } => { - match source_map.expr_syntax(call_expr) { - Ok(source_ptr) => acc.push( - MismatchedArgCount { call_expr: source_ptr, expected, found }.into(), - ), - Err(SyntheticSyntax) => (), - } - } BodyValidationDiagnostic::MissingMatchArms { match_expr } => { match source_map.expr_syntax(match_expr) { Ok(source_ptr) => { diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 71eb7e3995..335d95c0cf 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -17,7 +17,7 @@ use crate::{ deconstruct_pat::DeconstructedPat, usefulness::{compute_match_usefulness, MatchCheckCtx}, }, - InferenceResult, Interner, TyExt, + InferenceResult, TyExt, }; pub(crate) use hir_def::{ @@ -35,11 +35,6 @@ pub enum BodyValidationDiagnostic { ReplaceFilterMapNextWithFindMap { method_call_expr: ExprId, }, - MismatchedArgCount { - call_expr: ExprId, - expected: usize, - found: usize, - }, MissingMatchArms { match_expr: ExprId, }, @@ -119,18 +114,9 @@ impl ExprValidator { return; } - let is_method_call = matches!(expr, Expr::MethodCall { .. }); - let (sig, mut arg_count) = match expr { - Expr::Call { callee, args } => { - let callee = &self.infer.type_of_expr[*callee]; - let sig = match callee.callable_sig(db) { - Some(sig) => sig, - None => return, - }; - (sig, args.len()) - } - Expr::MethodCall { receiver, args, .. } => { - let (callee, subst) = match self.infer.method_resolution(call_id) { + match expr { + Expr::MethodCall { receiver, .. } => { + let (callee, _) = match self.infer.method_resolution(call_id) { Some(it) => it, None => return, }; @@ -148,53 +134,9 @@ impl ExprValidator { }, ); } - let receiver = &self.infer.type_of_expr[*receiver]; - if receiver.strip_references().is_unknown() { - // if the receiver is of unknown type, it's very likely we - // don't know enough to correctly resolve the method call. - // This is kind of a band-aid for #6975. - return; - } - - let sig = db.callable_item_signature(callee.into()).substitute(Interner, &subst); - - (sig, args.len() + 1) } _ => return, }; - - if sig.is_varargs { - return; - } - - if sig.legacy_const_generics_indices.is_empty() { - let mut param_count = sig.params().len(); - - if arg_count != param_count { - if is_method_call { - param_count -= 1; - arg_count -= 1; - } - self.diagnostics.push(BodyValidationDiagnostic::MismatchedArgCount { - call_expr: call_id, - expected: param_count, - found: arg_count, - }); - } - } else { - // With `#[rustc_legacy_const_generics]` there are basically two parameter counts that - // are allowed. - let count_non_legacy = sig.params().len(); - let count_legacy = sig.params().len() + sig.legacy_const_generics_indices.len(); - if arg_count != count_non_legacy && arg_count != count_legacy { - self.diagnostics.push(BodyValidationDiagnostic::MismatchedArgCount { - call_expr: call_id, - // Since most users will use the legacy way to call them, report against that. - expected: count_legacy, - found: arg_count, - }); - } - } } fn validate_match( diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs index 9a6795a1c8..71e81b4348 100644 --- a/crates/hir_ty/src/infer.rs +++ b/crates/hir_ty/src/infer.rs @@ -143,6 +143,7 @@ pub(crate) type InferResult = Result, TypeError>; pub enum InferenceDiagnostic { NoSuchField { expr: ExprId }, BreakOutsideOfLoop { expr: ExprId }, + MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, } /// A mismatch between an expected and an inferred type. diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs index 01b11861cd..238aff75c4 100644 --- a/crates/hir_ty/src/infer/expr.rs +++ b/crates/hir_ty/src/infer/expr.rs @@ -296,13 +296,18 @@ impl<'a> InferenceContext<'a> { break; } } + // if the function is unresolved, we use is_varargs=true to + // suppress the arg count diagnostic here + let is_varargs = + derefed_callee.callable_sig(self.db).map_or(false, |sig| sig.is_varargs) + || res.is_none(); let (param_tys, ret_ty) = match res { Some(res) => { let adjustments = auto_deref_adjust_steps(&derefs); self.write_expr_adj(*callee, adjustments); res } - None => (Vec::new(), self.err_ty()), + None => (Vec::new(), self.err_ty()), // FIXME diagnostic }; let indices_to_skip = self.check_legacy_const_generics(derefed_callee, args); self.register_obligations_for_call(&callee_ty); @@ -313,7 +318,14 @@ impl<'a> InferenceContext<'a> { param_tys.clone(), ); - self.check_call_arguments(args, &expected_inputs, ¶m_tys, &indices_to_skip); + self.check_call_arguments( + tgt_expr, + args, + &expected_inputs, + ¶m_tys, + &indices_to_skip, + is_varargs, + ); self.normalize_associated_types_in(ret_ty) } Expr::MethodCall { receiver, args, method_name, generic_args } => self @@ -948,22 +960,28 @@ impl<'a> InferenceContext<'a> { }; let method_ty = method_ty.substitute(Interner, &substs); self.register_obligations_for_call(&method_ty); - let (formal_receiver_ty, param_tys, ret_ty) = match method_ty.callable_sig(self.db) { - Some(sig) => { - if !sig.params().is_empty() { - (sig.params()[0].clone(), sig.params()[1..].to_vec(), sig.ret().clone()) - } else { - (self.err_ty(), Vec::new(), sig.ret().clone()) + let (formal_receiver_ty, param_tys, ret_ty, is_varargs) = + match method_ty.callable_sig(self.db) { + Some(sig) => { + if !sig.params().is_empty() { + ( + sig.params()[0].clone(), + sig.params()[1..].to_vec(), + sig.ret().clone(), + sig.is_varargs, + ) + } else { + (self.err_ty(), Vec::new(), sig.ret().clone(), sig.is_varargs) + } } - } - None => (self.err_ty(), Vec::new(), self.err_ty()), - }; + None => (self.err_ty(), Vec::new(), self.err_ty(), true), + }; self.unify(&formal_receiver_ty, &receiver_ty); let expected_inputs = self.expected_inputs_for_expected_output(expected, ret_ty.clone(), param_tys.clone()); - self.check_call_arguments(args, &expected_inputs, ¶m_tys, &[]); + self.check_call_arguments(tgt_expr, args, &expected_inputs, ¶m_tys, &[], is_varargs); self.normalize_associated_types_in(ret_ty) } @@ -996,11 +1014,21 @@ impl<'a> InferenceContext<'a> { fn check_call_arguments( &mut self, + expr: ExprId, args: &[ExprId], expected_inputs: &[Ty], param_tys: &[Ty], skip_indices: &[u32], + is_varargs: bool, ) { + if args.len() != param_tys.len() + skip_indices.len() && !is_varargs { + self.push_diagnostic(InferenceDiagnostic::MismatchedArgCount { + call_expr: expr, + expected: param_tys.len() + skip_indices.len(), + found: args.len(), + }); + } + // Quoting https://github.com/rust-lang/rust/blob/6ef275e6c3cb1384ec78128eceeb4963ff788dca/src/librustc_typeck/check/mod.rs#L3325 -- // We do this in a pretty awful way: first we type-check any arguments // that are not closures, then we type-check the closures. This is so @@ -1188,7 +1216,15 @@ impl<'a> InferenceContext<'a> { // only use legacy const generics if the param count matches with them if data.params.len() + data.legacy_const_generics_indices.len() != args.len() { - return Vec::new(); + if args.len() <= data.params.len() { + return Vec::new(); + } else { + // there are more parameters than there should be without legacy + // const params; use them + let mut indices = data.legacy_const_generics_indices.clone(); + indices.sort(); + return indices; + } } // check legacy const parameters From c2a31bfbb804b1e7a4ac51ea0f0d40f3ed05d017 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 27 Mar 2022 19:12:00 +0200 Subject: [PATCH 2/2] Remove legacy_const_generics_indices from CallableSig I want to remove CallableSig anyway, and it's not needed anymore. --- crates/hir_ty/src/lib.rs | 18 ++---------------- crates/hir_ty/src/lower.rs | 5 +---- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs index 945b4b0e4a..8729b52ae8 100644 --- a/crates/hir_ty/src/lib.rs +++ b/crates/hir_ty/src/lib.rs @@ -217,7 +217,6 @@ pub fn make_canonical>( pub struct CallableSig { params_and_return: Arc<[Ty]>, is_varargs: bool, - legacy_const_generics_indices: Arc<[u32]>, } has_interner!(CallableSig); @@ -228,11 +227,7 @@ pub type PolyFnSig = Binders; impl CallableSig { pub fn from_params_and_return(mut params: Vec, ret: Ty, is_varargs: bool) -> CallableSig { params.push(ret); - CallableSig { - params_and_return: params.into(), - is_varargs, - legacy_const_generics_indices: Arc::new([]), - } + CallableSig { params_and_return: params.into(), is_varargs } } pub fn from_fn_ptr(fn_ptr: &FnPointer) -> CallableSig { @@ -249,14 +244,9 @@ impl CallableSig { .map(|arg| arg.assert_ty_ref(Interner).clone()) .collect(), is_varargs: fn_ptr.sig.variadic, - legacy_const_generics_indices: Arc::new([]), } } - pub fn set_legacy_const_generics_indices(&mut self, indices: &[u32]) { - self.legacy_const_generics_indices = indices.into(); - } - pub fn to_fn_ptr(&self) -> FnPointer { FnPointer { num_binders: 0, @@ -287,11 +277,7 @@ impl Fold for CallableSig { ) -> Result { let vec = self.params_and_return.to_vec(); let folded = vec.fold_with(folder, outer_binder)?; - Ok(CallableSig { - params_and_return: folded.into(), - is_varargs: self.is_varargs, - legacy_const_generics_indices: self.legacy_const_generics_indices, - }) + Ok(CallableSig { params_and_return: folded.into(), is_varargs: self.is_varargs }) } } diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs index 41bb94c5d5..d0226cec26 100644 --- a/crates/hir_ty/src/lower.rs +++ b/crates/hir_ty/src/lower.rs @@ -1364,10 +1364,7 @@ fn fn_sig_for_fn(db: &dyn HirDatabase, def: FunctionId) -> PolyFnSig { .with_type_param_mode(ParamLoweringMode::Variable); let ret = ctx_ret.lower_ty(&data.ret_type); let generics = generics(db.upcast(), def.into()); - let mut sig = CallableSig::from_params_and_return(params, ret, data.is_varargs()); - if !data.legacy_const_generics_indices.is_empty() { - sig.set_legacy_const_generics_indices(&data.legacy_const_generics_indices); - } + let sig = CallableSig::from_params_and_return(params, ret, data.is_varargs()); make_binders(db, &generics, sig) }