Skip to content

Commit a2998a1

Browse files
timfennisclaude
andcommitted
fix(analyser): walk back scope.rs incompat filter to preserve runtime narrowing 🔄
The previous push pushed the incompat-Dynamic check from the analyser down to scope.rs, returning Binding::None for calls where no overload's params accepted the call's args. That works for trivially broken calls (returns_int("…") etc.) but breaks code that relies on the static type being wider than the runtime value — e.g. the common `for line in lines { line = line.split(...); line.remove(0); }` shape, where `line`'s static type widens to `Sequence<String>` via LUB even though the runtime value is always a `List<String>`. The stricter analyser dropped `remove(List, Int)` for being incompatible with `Sequence<String>`, blocking valid runtime-dispatchable code (see issue #143 for the underlying inference limitation). Move the check back to the analyser's Dynamic-LUB path: candidates still get filtered before contributing to the inferred return type (so we don't synthesise a misleading concrete return for a doomed call), but the candidate list flows through to the compiler/runtime intact so runtime narrowing still has a chance to succeed. Also reverts the `Map<Any, ()>` → `Map<Any, Any>` empty-value default since it was tied to the same overly-strict story. The default-value folding into `value_type` stays — strictly sound and orthogonal. Test updates: bug0022 now expects "mismatched types: found Any but expected String" (the call result widens to Any rather than blocking the call); 013_002 substring reverted to the runtime-side wording. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent c21253e commit a2998a1

4 files changed

Lines changed: 75 additions & 58 deletions

File tree

ndc_analyser/src/analyser.rs

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::HashMap;
22
use std::fmt::Debug;
33

4-
use crate::scope::{ScopeTree, TypeBinding};
4+
use crate::scope::{ScopeTree, TypeBinding, synthetic_vec_sig};
55
use itertools::{Itertools, izip};
66
use ndc_core::{StaticType, TypeSignature};
77
use ndc_lexer::Span;
@@ -464,16 +464,15 @@ impl Analyser {
464464
}
465465

466466
if let Some(default) = default {
467-
// The default value's type is what `map[missing]` returns,
468-
// so it contributes to the value type just like a regular
469-
// entry would.
467+
// The default is what `map[missing]` returns, so it
468+
// contributes to the value type just like a regular entry.
470469
let default_type = self.analyse_or_any(default);
471470
Self::fold_lub(&mut value_type, default_type);
472471
}
473472

474473
Ok(StaticType::Map {
475474
key: Box::new(key_type.unwrap_or(StaticType::Any)),
476-
value: Box::new(value_type.unwrap_or(StaticType::Any)),
475+
value: Box::new(value_type.unwrap_or_else(StaticType::unit)),
477476
})
478477
}
479478
Expression::Return { value } => {
@@ -542,20 +541,29 @@ impl Analyser {
542541
}
543542
}
544543
Binding::Dynamic(candidates) => {
545-
// Runtime decides which candidate fires. LUB across every
546-
// candidate's inferred return type — scalars contribute their
547-
// declared return, vecs contribute `Tuple<elem_return; max_len>`.
548-
// `LUB(Tuple<...>, scalar) = Any` in the type lattice, so the
549-
// mixed case naturally falls to Any; pure-scalar Dynamic
550-
// recovers the precision PR #140 had to pessimise.
551-
// `resolve_function_binding` filters the all_by_name fallback
552-
// by signature compatibility, so by the time we get here
553-
// every candidate could plausibly accept the call's args.
554-
let return_type = candidates
544+
// Runtime decides which candidate fires. When every candidate
545+
// is statically compatible with the call's args, LUB across
546+
// their inferred returns — scalars contribute their declared
547+
// return, vecs contribute `Tuple<elem_return; max_len>`. When
548+
// any candidate isn't compat (this Dynamic came from the
549+
// all_by_name fallback where a same-named binding exists but
550+
// its signature doesn't match), widen to Any so we don't
551+
// synthesise a precise return for a call that's likely to
552+
// fail at runtime. Runtime narrowing of supertype args may
553+
// still succeed; the imprecise Any keeps soundness without
554+
// rejecting code the runtime can dispatch.
555+
let all_compat = candidates
555556
.iter()
556-
.map(|c| candidate_return(&self.scope_tree, c, argument_types))
557-
.reduce(|a, b| a.lub(&b))
558-
.unwrap_or(StaticType::Any);
557+
.all(|c| candidate_is_compat(&self.scope_tree, c, argument_types));
558+
let return_type = if all_compat {
559+
candidates
560+
.iter()
561+
.map(|c| candidate_return(&self.scope_tree, c, argument_types))
562+
.reduce(|a, b| a.lub(&b))
563+
.unwrap_or(StaticType::Any)
564+
} else {
565+
StaticType::Any
566+
};
559567
StaticType::Function {
560568
parameters: None,
561569
return_type: Box::new(return_type),
@@ -934,6 +942,44 @@ fn static_vec_axis_len(sig: &[StaticType]) -> Option<usize> {
934942
})
935943
}
936944

945+
/// Whether a candidate's parameter types could accept the call's argument
946+
/// types. Used to gate the Dynamic-LUB so we don't synthesise a precise
947+
/// return for an all_by_name fallback that won't dispatch.
948+
///
949+
/// Examples (with `fn typed(s: String) -> Int` registered as `typed`):
950+
/// - scalar candidate for `typed`, call args `[Int]` → `false`
951+
/// - scalar candidate for `typed`, call args `[Any]` → `true`
952+
/// - scalar candidate for `+(Int, Int)`, call args `[Int]` → `false`
953+
/// (arity mismatch)
954+
fn candidate_is_compat(
955+
scope_tree: &ScopeTree,
956+
candidate: &Candidate,
957+
argument_types: &[StaticType],
958+
) -> bool {
959+
let StaticType::Function {
960+
parameters: Some(params),
961+
..
962+
} = scope_tree.get_type(candidate.var)
963+
else {
964+
return false;
965+
};
966+
if params.len() != argument_types.len() {
967+
return false;
968+
}
969+
let check_sig = if candidate.vectorized {
970+
match synthetic_vec_sig(argument_types) {
971+
Some(s) => s,
972+
None => return false,
973+
}
974+
} else {
975+
argument_types.to_vec()
976+
};
977+
params
978+
.iter()
979+
.zip(check_sig.iter())
980+
.all(|(p, a)| !p.is_incompatible_with(a))
981+
}
982+
937983
#[derive(thiserror::Error, Debug)]
938984
#[error("{text}")]
939985
pub struct AnalysisError {

ndc_analyser/src/scope.rs

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::fmt::{Debug, Formatter};
99
/// contributes the LUB of its elements; scalar and `Any` positions pass
1010
/// through unchanged so the candidate lookup stays permissive enough that
1111
/// runtime values which turn out to be tuples can still vec.
12-
fn synthetic_vec_sig(sig: &[StaticType]) -> Option<Vec<StaticType>> {
12+
pub(crate) fn synthetic_vec_sig(sig: &[StaticType]) -> Option<Vec<StaticType>> {
1313
let mut axis: Option<usize> = None;
1414
let mut vec_possible = false;
1515
for arg in sig {
@@ -563,36 +563,8 @@ impl ScopeTree {
563563
return Binding::Dynamic(combined);
564564
}
565565

566-
// Filter the all_by_name fallback by signature compatibility:
567-
// a concrete `Function<...>` slot whose parameters are
568-
// incompatible with the call's args is guaranteed to fail
569-
// dispatch at runtime, so leaving it in would let the analyser
570-
// synthesise a misleading concrete return for a doomed call.
571-
// `Any`-typed and variadic (`parameters: None`) slots stay — we
572-
// can't statically tell what those would accept.
573-
let filtered: Vec<ResolvedVar> = all_by_name
574-
.into_iter()
575-
.filter(|var| match self.get_type(*var) {
576-
StaticType::Function {
577-
parameters: Some(params),
578-
..
579-
} => {
580-
params.len() == sig.len()
581-
&& params
582-
.iter()
583-
.zip(sig)
584-
.all(|(p, a)| !p.is_incompatible_with(a))
585-
}
586-
StaticType::Function {
587-
parameters: None, ..
588-
}
589-
| StaticType::Any => true,
590-
_ => false,
591-
})
592-
.collect();
593-
594-
if !filtered.is_empty() {
595-
return Binding::Dynamic(filtered.into_iter().map(Candidate::scalar).collect());
566+
if !all_by_name.is_empty() {
567+
return Binding::Dynamic(all_by_name.into_iter().map(Candidate::scalar).collect());
596568
}
597569
Binding::None
598570
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
// expect-error: No function called '+' found that matches the arguments
1+
// expect-error: no function called '+' found matches the arguments
22
(1,2) + (5,3,2)
Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
// expect-error: No function called 'returns_int' found that matches the arguments 'Int'
1+
// expect-error: mismatched types: found Any but expected String
22
// `returns_int(42)` has no compatible overload — the only `returns_int`
3-
// declared takes a `String`. Before the fix, `resolve_function_binding`
4-
// fell back to all-by-name and emitted `Binding::Dynamic([returns_int])`
5-
// despite the type mismatch; the analyser then LUB'd its declared return
6-
// (`Int`) and reported "found Int but expected …" at downstream
7-
// assignments, masking the real bug. Now the fallback filters concrete-
8-
// Function slots whose params don't match the call, leaving the binding
9-
// empty so the call surfaces at compile time as "no function found".
3+
// declared takes a `String`. Before the fix, the analyser LUB'd every
4+
// candidate's declared return regardless of compatibility, so this call
5+
// was inferred as `Int` and the assignment errored with the misleading
6+
// "found Int but expected String". The Dynamic-LUB now drops to Any
7+
// when any candidate's params can't accept the call's args, so the
8+
// type mismatch surfaces against `Any` instead.
109
fn returns_int(s: String) -> Int { 1 };
1110
let x: String = returns_int(42);

0 commit comments

Comments
 (0)