Skip to content

Commit 24937e5

Browse files
stroxlermeta-codesync[bot]
authored andcommitted
Make call boundaries own deferred quantified finishing
Summary: Deferred finishing should be boundary-scoped and explicit, not reachability-driven. This change makes `CallContext` tracking the source of truth for which fresh quantified vars must be finished, defers Forall-instantiated vars during call analysis, and keeps eager finishing only for ad-hoc subset checks outside calls. Also, we're now able to clean up a good chunk of code since the new helpers added in D103328383 are now used uniformly. This fixes a number of regressions that were related to the incorrect early finishing of generics. Reviewed By: rchen152 Differential Revision: D103337567 fbshipit-source-id: 808e848b890b744386b87d89b22c70d20638c4bb
1 parent 19783b8 commit 24937e5

5 files changed

Lines changed: 42 additions & 104 deletions

File tree

pyrefly/lib/solver/solver.rs

Lines changed: 18 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -2596,96 +2596,25 @@ impl Solver {
25962596
.collect();
25972597
let mut roots: SmallSet<Var> = vs.0.into_iter().collect();
25982598
roots.extend(tracked_fresh_vars.0);
2599-
// Forall instantiation during call analysis can unify call-scope vars
2600-
// with additional fresh vars that are only visible through Answer /
2601-
// ResidualAnswer payloads. Finish the full reachable closure so no
2602-
// reachable Variable::Quantified can leak to pinning.
2603-
//
2604-
// We also must include reachable vars that already became Answer but
2605-
// still have pending instantiation errors. Those errors are surfaced by
2606-
// finish_quantified, so excluding Answer vars here can silently drop
2607-
// call-site specialization diagnostics.
2608-
let roots = roots.into_iter().collect::<Vec<_>>();
2609-
let mut already_finished: SmallSet<Var> = SmallSet::new();
2610-
loop {
2611-
// Fixed-point: finishing can mutate solver state and expose new
2612-
// reachable vars that also require finishing.
2613-
let reachable_finish_vars = self.reachable_finish_vars_from_roots(&roots);
2614-
let mut next_round: Vec<Var> = reachable_finish_vars
2615-
.into_iter()
2616-
.filter(|var| !already_finished.contains(var))
2617-
.collect();
2618-
// Payload-driven overload pruning must consider solved vars even if
2619-
// they already collapsed to `Answer` before finishing and therefore
2620-
// are not selected by reachability-based finishing alone.
2621-
next_round.extend(
2622-
payload_vars
2623-
.iter()
2624-
.copied()
2625-
.filter(|var| !already_finished.contains(var)),
2626-
);
2627-
next_round.sort_unstable();
2628-
next_round.dedup();
2629-
if next_round.is_empty() {
2630-
break;
2631-
}
2632-
already_finished.extend(next_round.iter().copied());
2633-
let mut subset = self.subset(type_order);
2634-
self.finish_quantified_with_subset_and_payloads(
2635-
QuantifiedHandle(next_round),
2636-
infer_with_first_use,
2637-
&mut |got, want| subset.is_subset_eq_probe_for_pruning(got, want),
2638-
Some(&overload_witness_payloads),
2639-
)?;
2640-
}
2641-
Ok(())
2642-
}
2643-
2644-
fn reachable_finish_vars_from_roots(&self, roots: &[Var]) -> SmallSet<Var> {
2645-
if roots.is_empty() {
2646-
return SmallSet::new();
2647-
}
2648-
let variables = self.variables.lock();
2649-
let instantiation_errors = self.instantiation_errors.read();
2650-
let mut visited: SmallSet<Var> = SmallSet::new();
2651-
let mut reachable_finish_vars: SmallSet<Var> = SmallSet::new();
2652-
let mut stack = roots.to_vec();
2653-
while let Some(var) = stack.pop() {
2654-
if !visited.insert(var) {
2655-
continue;
2656-
}
2657-
let variable = variables.get(var);
2658-
let needs_finish = match &*variable {
2659-
Variable::Quantified { .. } => true,
2660-
Variable::Answer(_) | Variable::ResidualAnswer { .. } => {
2661-
instantiation_errors.contains_key(&var)
2662-
}
2663-
_ => false,
2664-
};
2665-
if needs_finish {
2666-
reachable_finish_vars.insert(var);
2667-
}
2668-
match &*variable {
2669-
Variable::Answer(ty) => {
2670-
stack.extend(ty.collect_maybe_placeholder_vars());
2671-
}
2672-
Variable::ResidualAnswer { target_vars: _, ty } => {
2673-
// `target_vars` are read-gates for residual visibility,
2674-
// not ownership edges for finishing reachability.
2675-
stack.extend(ty.collect_maybe_placeholder_vars());
2676-
}
2677-
Variable::Quantified { .. } => {}
2678-
Variable::Unwrap(bounds) => {
2679-
for ty in bounds.lower.iter().chain(bounds.upper.iter()) {
2680-
stack.extend(ty.collect_maybe_placeholder_vars());
2681-
}
2682-
}
2683-
Variable::PartialQuantified(_)
2684-
| Variable::PartialContained(_)
2685-
| Variable::Recursive => {}
2686-
}
2599+
// Solve boundaries explicitly own fresh quantified tracking. We finish
2600+
// the exact boundary set (explicit roots + fresh vars + payload vars),
2601+
// rather than using reachability expansion that can miss or overreach.
2602+
let mut all_boundary_vars: Vec<Var> = roots.into_iter().collect();
2603+
// Payload-driven overload pruning must include solved vars even if they
2604+
// already collapsed to `Answer` before boundary finishing.
2605+
all_boundary_vars.extend(payload_vars);
2606+
all_boundary_vars.sort_unstable();
2607+
all_boundary_vars.dedup();
2608+
if all_boundary_vars.is_empty() {
2609+
return Ok(());
26872610
}
2688-
reachable_finish_vars
2611+
let mut subset = self.subset(type_order);
2612+
self.finish_quantified_with_subset_and_payloads(
2613+
QuantifiedHandle(all_boundary_vars),
2614+
infer_with_first_use,
2615+
&mut |got, want| subset.is_subset_eq_probe_for_pruning(got, want),
2616+
Some(&overload_witness_payloads),
2617+
)
26892618
}
26902619

26912620
/// Finish all quantified vars reachable from `ty` using the solver default

pyrefly/lib/solver/subset.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2246,11 +2246,14 @@ impl<'a, Ans: LookupAnswer> Subset<'a, Ans> {
22462246
),
22472247
(Type::Forall(forall), _) => {
22482248
let forall_type = Type::Forall(forall.clone());
2249-
// Finalizing the quantified vars returns instantiation errors
2249+
// Instantiate once, then either defer finishing to the active
2250+
// call boundary (when inside call analysis) or finish eagerly
2251+
// for ad-hoc subset checks outside calls.
22502252
let (vs, got) = self.type_order.instantiate_fresh_forall((**forall).clone());
22512253
self.active_call_context
22522254
.register_fresh_quantified_vars(vs.vars());
22532255
let argument_side = self.active_argument_side();
2256+
let in_call_analysis = !matches!(argument_side, ArgumentSide::NotAnalyzingACall);
22542257
let witness = self.make_forall_witness(&forall_type, &vs, want);
22552258
let (result, mut maybe_witness) =
22562259
self.with_active_witness_and_side(witness, argument_side, |me| {
@@ -2267,15 +2270,24 @@ impl<'a, Ans: LookupAnswer> Subset<'a, Ans> {
22672270
}
22682271
self.solver.record_generic_residuals_for_witness(witness);
22692272
}
2270-
result.and(
2271-
self.solver
2273+
if in_call_analysis {
2274+
result
2275+
} else {
2276+
// Even when subset checking fails, finish the fresh vars
2277+
// to avoid leaking Quantified placeholders in ad-hoc paths.
2278+
let finish_result = self
2279+
.solver
22722280
.finish_quantified_with_subset(
22732281
vs,
22742282
self.solver.infer_with_first_use,
22752283
&mut |got, want| self.is_subset_eq_probe_for_pruning(got, want),
22762284
)
2277-
.map_err(SubsetError::TypeVarSpecialization),
2278-
)
2285+
.map_err(SubsetError::TypeVarSpecialization);
2286+
match result {
2287+
Ok(()) => finish_result,
2288+
Err(e) => Err(e),
2289+
}
2290+
}
22792291
}
22802292
(_, Type::Forall(forall)) => self.is_subset_eq(got, &forall.body.clone().as_type()),
22812293
(Type::TypeVar(l), Type::TypeVar(u)) => {

pyrefly/lib/test/calls.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,10 @@ f() # E: Deprecated
226226
);
227227

228228
testcase!(
229-
bug = "There's a bug in generic overload resolution where we finish too early",
230229
test_reduce_call,
231230
r#"
232231
from functools import reduce
233-
reduce(max, [1,2]) # E: Overload type was not compatible with solved type variables: _T = int
232+
reduce(max, [1,2])
234233
"#,
235234
);
236235

pyrefly/lib/test/constructors.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -820,18 +820,17 @@ takes_Cstr_wrong(C) # E: Argument `type[C]` is not assignable to parameter `x` w
820820
);
821821

822822
testcase!(
823-
bug = "Early finishing of generic residuals is causing false positives here",
824823
test_init_to_callable_generics,
825824
r#"
826825
from typing import Generic, TypeVar, assert_type, Callable
827826
T = TypeVar("T")
828827
class C(Generic[T]):
829828
def __init__[V](self: "C[V]", x: V) -> None: pass
830829
def takes_callable[V](x: Callable[[V], C[V]], y: V) -> C[V]: ...
831-
out1 = takes_callable(C, 42) # E: Argument `Literal[42]` is not assignable to parameter `y` with type `GenericResidual@V` in function `takes_callable`
832-
assert_type(out1, C[int]) # E: assert_type(C[GenericResidual@V], C[int]) failed
833-
out2 = takes_callable(C, "hello") # E: Argument `Literal['hello']` is not assignable to parameter `y` with type `GenericResidual@V` in function `takes_callable`
834-
assert_type(out2, C[str]) # E: assert_type(C[GenericResidual@V], C[str]) failed
830+
out1 = takes_callable(C, 42)
831+
assert_type(out1, C[int])
832+
out2 = takes_callable(C, "hello")
833+
assert_type(out2, C[str])
835834
"#,
836835
);
837836

pyrefly/lib/test/generic_restrictions.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -668,14 +668,13 @@ assert_type(f(), int)
668668
);
669669

670670
testcase!(
671-
bug = "Early finishing of generic residuals is causing false positives here",
672671
test_arg_against_typevar_bound,
673672
r#"
674673
from typing import Callable, Iterable
675674
def reduce[_S](function: Callable[[_S, _S], _S], iterable: Iterable[_S]) -> _S: ...
676675
def f[_T: str](arg1: _T, arg2: _T) -> _T: ...
677-
reduce(f, [1]) # E: Argument `list[int]` is not assignable to parameter `iterable` with type `Iterable[GenericResidual@_T]` in function `reduce`
678-
reduce(f, ["ok"]) # E: Argument `list[str]` is not assignable to parameter `iterable` with type `Iterable[GenericResidual@_T]` in function `reduce`
676+
reduce(f, [1]) # E: `int` is not assignable to upper bound `str` of type variable `_T`
677+
reduce(f, ["ok"])
679678
"#,
680679
);
681680

0 commit comments

Comments
 (0)