Skip to content

Commit 24aa558

Browse files
timfennisclaude
andcommitted
fix(analyser): pick most-specific overload in scope 🎯
Same-scope overload resolution used `rposition`, so a later-registered less-specific overload would shadow an earlier more-specific one. The `locate` stdlib function exposed this: passing a predicate dispatched to the element-equality overload (registered second), which compared the function value against each element and produced "locate did not find anything". `Scope::find_function` now collects every signature-compatible candidate and returns the one not strictly dominated on its parameter signature in the subtype partial order, tie-breaking by latest registration. Variadic overloads are treated as least specific. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5c896d4 commit 24aa558

2 files changed

Lines changed: 230 additions & 2 deletions

File tree

‎ndc_analyser/src/scope.rs‎

Lines changed: 223 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,27 @@ struct ScalarWalk {
4747
all_by_name: Vec<ResolvedVar>,
4848
}
4949

50+
/// Returns `true` iff `more` is a strict subtype of `less` pointwise — i.e.
51+
/// every position satisfies `more[i] <: less[i]` and at least one position is
52+
/// not bidirectionally equivalent. Used by [`Scope::find_function`] to order
53+
/// matching overloads by specificity: an overload with `more` parameters
54+
/// accepts strictly fewer call signatures than one with `less` parameters.
55+
fn is_strictly_more_specific(more: &[StaticType], less: &[StaticType]) -> bool {
56+
if more.len() != less.len() {
57+
return false;
58+
}
59+
let mut any_strict = false;
60+
for (m, l) in more.iter().zip(less) {
61+
if !m.is_subtype(l) {
62+
return false;
63+
}
64+
if !l.is_subtype(m) {
65+
any_strict = true;
66+
}
67+
}
68+
any_strict
69+
}
70+
5071
/// If every per-position candidate list contains exactly one entry and they
5172
/// all point to the same scalar overload, return it. This is the only case
5273
/// where `Binding::Resolved(Candidate::Vec)` is safe to emit: a single scalar
@@ -205,10 +226,71 @@ impl Scope {
205226
.map(|idx| idx + self.base_offset)
206227
.collect()
207228
}
229+
/// Find the best-matching overload in this scope for a call of the given
230+
/// signature, or `None` if no overload in this scope accepts the call.
231+
///
232+
/// Resolution proceeds in two stages:
233+
/// 1. Filter to overloads whose declared signature accepts `find_types`
234+
/// (the usual subtype check via [`StaticType::is_fn_and_matches`]).
235+
/// 2. Among the survivors, pick the one whose parameter signature is
236+
/// most specific — i.e. not strictly dominated by any other match's
237+
/// parameter signature in the subtype partial order. A signature `b`
238+
/// strictly dominates `a` when `b <: a` and `a` is not `<: b`.
239+
///
240+
/// Variadic overloads (`parameters: None`) match anything but are treated
241+
/// as the least specific shape: any concrete overload that also matches
242+
/// will dominate them.
243+
///
244+
/// When multiple undominated overloads remain (genuinely incomparable
245+
/// shapes, e.g. `fn(Int, Any)` vs `fn(Any, Int)` called with `(Int, Int)`),
246+
/// the latest-registered one wins — preserving shadow semantics.
208247
fn find_function(&self, find_ident: &str, find_types: &[StaticType]) -> Option<usize> {
209-
self.identifiers
248+
let matches: Vec<usize> = self
249+
.identifiers
210250
.iter()
211-
.rposition(|b| b.name == find_ident && b.binding.typ().is_fn_and_matches(find_types))
251+
.enumerate()
252+
.filter_map(|(idx, b)| {
253+
(b.name == find_ident && b.binding.typ().is_fn_and_matches(find_types))
254+
.then_some(idx)
255+
})
256+
.collect();
257+
258+
if matches.is_empty() {
259+
return None;
260+
}
261+
262+
let params_of = |idx: usize| -> Option<&[StaticType]> {
263+
match self.identifiers[idx].binding.typ() {
264+
StaticType::Function {
265+
parameters: Some(p),
266+
..
267+
} => Some(p.as_slice()),
268+
// Variadic / non-function bindings have no concrete shape.
269+
_ => None,
270+
}
271+
};
272+
273+
// Iterate latest-first so ties resolve to the most-recently registered overload.
274+
matches
275+
.iter()
276+
.rev()
277+
.copied()
278+
.find(|&i| {
279+
let Some(pi) = params_of(i) else {
280+
// Variadic only wins if every other match is also variadic.
281+
return matches.iter().all(|&j| params_of(j).is_none());
282+
};
283+
matches.iter().all(|&j| {
284+
if i == j {
285+
return true;
286+
}
287+
let Some(pj) = params_of(j) else {
288+
// Variadic never dominates a concrete overload.
289+
return true;
290+
};
291+
!is_strictly_more_specific(pj, pi)
292+
})
293+
})
212294
.map(|idx| idx + self.base_offset)
213295
}
214296

@@ -1208,4 +1290,143 @@ mod tests {
12081290
let middle_idx = tree.current_scope_idx;
12091291
assert_eq!(tree.scopes[middle_idx].upvalues.len(), 1);
12101292
}
1293+
1294+
fn fn_type(params: Vec<StaticType>, ret: StaticType) -> StaticType {
1295+
StaticType::Function {
1296+
parameters: Some(params),
1297+
return_type: Box::new(ret),
1298+
}
1299+
}
1300+
1301+
fn variadic_fn(ret: StaticType) -> StaticType {
1302+
StaticType::Function {
1303+
parameters: None,
1304+
return_type: Box::new(ret),
1305+
}
1306+
}
1307+
1308+
fn assert_resolves_to(tree: &mut ScopeTree, name: &str, sig: &[StaticType], slot: usize) {
1309+
let resolved = tree.resolve_call(name, sig, CallKind::Regular);
1310+
match resolved.binding {
1311+
Binding::Resolved(Candidate::Scalar(ResolvedVar::Global { slot: s })) => {
1312+
assert_eq!(s, slot, "expected global slot {slot} for {name}, got {s}");
1313+
}
1314+
Binding::Resolved(Candidate::Scalar(ResolvedVar::Local { slot: s })) => {
1315+
assert_eq!(s, slot, "expected local slot {slot} for {name}, got {s}");
1316+
}
1317+
other => panic!("expected Resolved Scalar, got {other:?}"),
1318+
}
1319+
}
1320+
1321+
// Two overloads of `locate` registered in the same scope: a more-specific
1322+
// predicate version (takes a function) and a less-specific element version
1323+
// (takes any value). When called with a function value, the predicate
1324+
// version must win even though the element version was registered later.
1325+
#[test]
1326+
fn more_specific_overload_wins_in_same_scope() {
1327+
let seq_any = StaticType::Sequence(Box::new(StaticType::Any));
1328+
let fn_any = variadic_fn(StaticType::Any);
1329+
let mut tree = ScopeTree::from_global_scope(vec![
1330+
(
1331+
"locate".into(),
1332+
fn_type(vec![seq_any.clone(), fn_any.clone()], StaticType::Any),
1333+
),
1334+
(
1335+
"locate".into(),
1336+
fn_type(vec![seq_any.clone(), StaticType::Any], StaticType::Any),
1337+
),
1338+
]);
1339+
1340+
let call_sig = [
1341+
StaticType::List(Box::new(StaticType::Int)),
1342+
fn_type(vec![StaticType::Any], StaticType::Bool),
1343+
];
1344+
// Slot 0 is the predicate version; it must win over slot 1 (element).
1345+
assert_resolves_to(&mut tree, "locate", &call_sig, 0);
1346+
}
1347+
1348+
// From the spec: distance(fn(Int) -> String, fn(Number) -> String) = 1.
1349+
// When two overloads in the same scope both match, the one whose params
1350+
// are a strict subtype (here `Int` is a subtype of `Number`) wins.
1351+
#[test]
1352+
fn more_specific_numeric_overload_wins() {
1353+
let mut tree = ScopeTree::from_global_scope(vec![
1354+
(
1355+
"foo".into(),
1356+
fn_type(vec![StaticType::Number], StaticType::String),
1357+
),
1358+
(
1359+
"foo".into(),
1360+
fn_type(vec![StaticType::Int], StaticType::String),
1361+
),
1362+
]);
1363+
assert_resolves_to(&mut tree, "foo", &[StaticType::Int], 1);
1364+
}
1365+
1366+
// `fn foo(Any)` declared in an inner scope matches every call shape, so
1367+
// it completely shadows any `foo` overload in the parent scope — even a
1368+
// more-specific one. The walk must stop at the first scope with a match.
1369+
#[test]
1370+
fn inner_any_overload_shadows_parent() {
1371+
let mut tree = ScopeTree::from_global_scope(vec![(
1372+
"foo".into(),
1373+
fn_type(vec![StaticType::Int], StaticType::Int),
1374+
)]);
1375+
1376+
// Push an inner scope holding `foo(Any) -> Any`.
1377+
tree.new_block_scope();
1378+
let inner_var = tree.create_local_binding(
1379+
"foo".into(),
1380+
TypeBinding::Inferred(fn_type(vec![StaticType::Any], StaticType::Any)),
1381+
);
1382+
let ResolvedVar::Local { slot: inner_slot } = inner_var else {
1383+
panic!("expected local binding");
1384+
};
1385+
1386+
// Called with `Int`: inner `foo(Any)` matches and shadows the global.
1387+
let resolved = tree.resolve_call("foo", &[StaticType::Int], CallKind::Regular);
1388+
match resolved.binding {
1389+
Binding::Resolved(Candidate::Scalar(ResolvedVar::Local { slot })) => {
1390+
assert_eq!(
1391+
slot, inner_slot,
1392+
"inner foo(Any) must shadow global foo(Int)"
1393+
);
1394+
}
1395+
other => panic!("expected inner local resolution, got {other:?}"),
1396+
}
1397+
}
1398+
1399+
// `fn foo(Int)` in an inner scope only matches `Int` calls; calls with a
1400+
// different argument type must fall through to the parent scope's overload.
1401+
#[test]
1402+
fn inner_int_overload_does_not_shadow_other_types() {
1403+
let mut tree = ScopeTree::from_global_scope(vec![(
1404+
"foo".into(),
1405+
fn_type(vec![StaticType::Any], StaticType::Int),
1406+
)]);
1407+
1408+
tree.new_block_scope();
1409+
tree.create_local_binding(
1410+
"foo".into(),
1411+
TypeBinding::Inferred(fn_type(vec![StaticType::Int], StaticType::Int)),
1412+
);
1413+
1414+
// Called with `String`: inner `foo(Int)` doesn't match, falls through
1415+
// to the global `foo(Any)`.
1416+
assert_resolves_to(&mut tree, "foo", &[StaticType::String], 0);
1417+
}
1418+
1419+
// A concrete overload should beat a variadic overload registered in the
1420+
// same scope when both match the call.
1421+
#[test]
1422+
fn concrete_overload_beats_variadic() {
1423+
let mut tree = ScopeTree::from_global_scope(vec![
1424+
("foo".into(), variadic_fn(StaticType::Any)),
1425+
(
1426+
"foo".into(),
1427+
fn_type(vec![StaticType::Int], StaticType::Int),
1428+
),
1429+
]);
1430+
assert_resolves_to(&mut tree, "foo", &[StaticType::Int], 1);
1431+
}
12111432
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Element-equality overload: find the index of a value.
2+
assert_eq(locate([1, 2, 3], 2), 1);
3+
assert_eq(locate(["a", "b", "c"], "c"), 2);
4+
5+
// Predicate overload: find the first index matching a function.
6+
assert_eq(locate([1, 2, 3], fn(x) => x == 2), 1);
7+
assert_eq(locate([10, 20, 30, 40], fn(x) => x > 25), 2);

0 commit comments

Comments
 (0)