Skip to content

Sync function call args check fudging with rustc #22092

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.

Sign up for GitHub

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
ShoyuVanilla merged 1 commit into from
Apr 19, 2026
+23 −8
Merged

Sync function call args check fudging with rustc #22092

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 23 additions & 8 deletions crates/hir-ty/src/infer/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ use crate::{
lower::{GenericPredicates, lower_mutability},
method_resolution::{self, CandidateId, MethodCallee, MethodError},
next_solver::{
ErrorGuaranteed, FnSig, GenericArg, GenericArgs, TraitRef, Ty, TyKind, TypeError,
ClauseKind, ErrorGuaranteed, FnSig, GenericArg, GenericArgs, TraitRef, Ty, TyKind,
TypeError,
infer::{
BoundRegionConversionTime, InferOk,
traits::{Obligation, ObligationCause},
Expand Down Expand Up @@ -1898,6 +1899,15 @@ impl<'db> InferenceContext<'_, 'db> {
// Whether the function is variadic, for example when imported from C
c_variadic: bool,
) {
let formal_input_tys: Vec<_> = formal_input_tys
.iter()
.map(|&ty| {
let generalized_ty = self.table.next_ty_var();
let _ = self.demand_eqtype(call_expr.into(), ty, generalized_ty);
Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting, if you'd demand supertype/subtype I would understand, but isn't demand_eqtype equivalent to setting the type var?

Copy link
Copy Markdown
Member Author

@ShoyuVanilla ShoyuVanilla Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a bit subtle thing. Relating a type with a ty var generalizes it, which entails normalization:

rust-analyzer/crates/hir-ty/src/next_solver/infer/relate/generalize.rs

Line 42 in 848e6aa

pub fn instantiate_ty_var<R: PredicateEmittingRelation<InferCtxt<'db>>>(

generalized_ty
})
.collect();

// First, let's unify the formal method signature with the expectation eagerly.
// We use this to guide coercion inference; it's output is "fudged" which means
// any remaining type variables are assigned to new, unrelated variables. This
Expand All @@ -1917,18 +1927,23 @@ impl<'db> InferenceContext<'_, 'db> {
// No argument expectations are produced if unification fails.
let origin = ObligationCause::new();
ocx.sup(&origin, self.table.param_env, expected_output, formal_output)?;

for &ty in &formal_input_tys {
ocx.register_obligation(Obligation::new(
Copy link
Copy Markdown
Contributor

@ChayimFriedman2 ChayimFriedman2 Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have self.table.register_wf_obligation(), for short.

Copy link
Copy Markdown
Member Author

@ShoyuVanilla ShoyuVanilla Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I forgot about it 😅 Thanks!

Copy link
Copy Markdown
Member Author

@ShoyuVanilla ShoyuVanilla Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, I think we shouldn't use it, since we are probing in this context so we shouldn't leave anything to the table.fulfillment_cx when leaving out of the probe. That's why we are creating a fresh new let ocx = ObligationCtxt::new(..) in this scope

self.interner(),
ObligationCause::new(),
self.table.param_env,
ClauseKind::WellFormed(ty.into()),
));
}

if !ocx.try_evaluate_obligations().is_empty() {
return Err(TypeError::Mismatch);
}

// Record all the argument types, with the args
// produced from the above subtyping unification.
Ok(Some(
formal_input_tys
.iter()
.map(|&ty| self.table.infer_ctxt.resolve_vars_if_possible(ty))
.collect(),
))
Ok(Some(formal_input_tys.clone()))
Copy link
Copy Markdown
Member Author

@ShoyuVanilla ShoyuVanilla Apr 19, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't (yet) applied to rustc, but as we call resolve_vars_if_possible in fudge_inference_if_ok, no need to do it again here

rust-analyzer/crates/hir-ty/src/next_solver/infer/snapshot/fudge.rs

Line 105 in fba71bb

Ok((snapshot_vars, self.resolve_vars_if_possible(value)))

View changes since the review

})
.ok()
})
Expand All @@ -1939,7 +1954,7 @@ impl<'db> InferenceContext<'_, 'db> {
assert_eq!(expected_input_tys.len(), formal_input_tys.len());
expected_input_tys
} else {
formal_input_tys
&formal_input_tys
};

let minimum_input_count = expected_input_tys.len();
Expand Down