Skip to content

Commit

Permalink
Merge #11833
Browse files Browse the repository at this point in the history
11833: internal: Move mismatched arg count diagnostic to inference r=flodiebold a=flodiebold

This means we only need to handle legacy const generics in one place, and it fits there especially since there will be more diagnostics coming.

Co-authored-by: Florian Diebold <flodiebold@gmail.com>
  • Loading branch information
bors[bot] and flodiebold committed Mar 27, 2022
2 parents 837901a + c2a31bf commit bc08b8e
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 103 deletions.
21 changes: 13 additions & 8 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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) => {
Expand Down
66 changes: 4 additions & 62 deletions crates/hir_ty/src/diagnostics/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
deconstruct_pat::DeconstructedPat,
usefulness::{compute_match_usefulness, MatchCheckCtx},
},
InferenceResult, Interner, TyExt,
InferenceResult, TyExt,
};

pub(crate) use hir_def::{
Expand All @@ -35,11 +35,6 @@ pub enum BodyValidationDiagnostic {
ReplaceFilterMapNextWithFindMap {
method_call_expr: ExprId,
},
MismatchedArgCount {
call_expr: ExprId,
expected: usize,
found: usize,
},
MissingMatchArms {
match_expr: ExprId,
},
Expand Down Expand Up @@ -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,
};
Expand All @@ -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(
Expand Down
1 change: 1 addition & 0 deletions crates/hir_ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, 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.
Expand Down
62 changes: 49 additions & 13 deletions crates/hir_ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -313,7 +318,14 @@ impl<'a> InferenceContext<'a> {
param_tys.clone(),
);

self.check_call_arguments(args, &expected_inputs, &param_tys, &indices_to_skip);
self.check_call_arguments(
tgt_expr,
args,
&expected_inputs,
&param_tys,
&indices_to_skip,
is_varargs,
);
self.normalize_associated_types_in(ret_ty)
}
Expr::MethodCall { receiver, args, method_name, generic_args } => self
Expand Down Expand Up @@ -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, &param_tys, &[]);
self.check_call_arguments(tgt_expr, args, &expected_inputs, &param_tys, &[], is_varargs);
self.normalize_associated_types_in(ret_ty)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 2 additions & 16 deletions crates/hir_ty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ pub fn make_canonical<T: HasInterner<Interner = Interner>>(
pub struct CallableSig {
params_and_return: Arc<[Ty]>,
is_varargs: bool,
legacy_const_generics_indices: Arc<[u32]>,
}

has_interner!(CallableSig);
Expand All @@ -228,11 +227,7 @@ pub type PolyFnSig = Binders<CallableSig>;
impl CallableSig {
pub fn from_params_and_return(mut params: Vec<Ty>, 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 {
Expand All @@ -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,
Expand Down Expand Up @@ -287,11 +277,7 @@ impl Fold<Interner> for CallableSig {
) -> Result<Self::Result, E> {
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 })
}
}

Expand Down
5 changes: 1 addition & 4 deletions crates/hir_ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit bc08b8e

Please sign in to comment.