Lambda type aliases: report arity mismatch instead of silent fallthrough#1301
Lambda type aliases: report arity mismatch instead of silent fallthrough#1301MikaelMayer wants to merge 29 commits into
Conversation
…original type (#1239) Add LMonoTy.checkAliasArity that detects when a type constructor name matches a registered alias but with the wrong number of type arguments. Use it in tconsAlias and aliasDef? to produce a clear error message instead of silently returning the unresolved type. The pure tconsAliasSimple and resolveAliases functions are unchanged to avoid impacting the many proofs that depend on their structure.
|
🤖⛏️ Design decisions:
|
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
MikaelMayer
left a comment
There was a problem hiding this comment.
🤖🔍 Clean, well-scoped PR. The checkAliasArity helper is a good design choice — it's pure, reusable across both aliasDef? and tconsAlias, and the proof adaptations are minimal and correct. The updated test legitimately demonstrates the bug fix (silent fallthrough → clear error), and the new test covers the scenario from the issue.
One observation: LMonoTy.resolveAliases still uses tconsAliasSimple directly without the arity check. The PR description explicitly states this is intentional to minimize proof impact, which is reasonable given the heavy proof machinery around resolveAliases. Worth noting for future work if users hit the same silent fallthrough via that path.
| (aliases : List TypeAlias) : Except Format Unit := | ||
| match aliases.find? (fun a => a.name == name) with | ||
| | none => .ok () | ||
| | some alias => |
There was a problem hiding this comment.
🤖🔍 Nit: find? returns the first alias matching by name. If the alias list ever contains multiple entries with the same name but different arities (e.g., overloaded aliases), the error message will report the arity of whichever appears first, which may not be the one the user intended. Consider whether the error message should list all registered arities for that name (e.g., "expected 1 or 3 type argument(s)").
Context
This is a minor edge case — if aliases are unique by name (which seems likely given the AliasesWF invariant), this is a non-issue. But if overloading by arity is ever supported, this would produce a confusing message.
There was a problem hiding this comment.
🤖 Good observation. As you noted, aliases are unique by name in the current system — TContext.AliasesWF ensures per-alias well-formedness, and the alias registration path does not allow duplicate names. The tconsAlias function already matches by name+arity, so checkAliasArity is only reached when no exact match exists. If overloading by arity is ever supported, we would need to revisit this, but for now the single-arity message is correct.
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
…pe-aliases-aliasdef-tconsalias
Fixes #1239
Summary
aliasDef?andtconsAliasnow report a clear arity mismatch error when a type constructor name matches a registered alias but with the wrong number of type arguments. Previously, this case was silently treated as "no matching alias" and the original unresolved type was returned, leading to confusing downstream type errors.Approach
Added
LMonoTy.checkAliasAritywhich looks up aliases by name only and checks arity. It's called intconsAlias(in thenonebranch, after the name+arity find fails) and inaliasDef?(before callingtconsAliasSimple). The puretconsAliasSimpleandresolveAliasesfunctions are unchanged to minimize proof impact.Testing
Existing tests pass. Updated one test that was demonstrating the bug (silently returning the unresolved type on arity mismatch) to expect the new error. Added a new test exercising the arity mismatch scenario from the issue.