You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
engine: variable-vs-dimension name collision is never refused at definition; each spelling resolves it differently (plain read accepted, PREVIOUS/INIT refused, bare read silently yields the element index) #1050
A variable whose canonical name equals a dimension's canonical name (other vs Other) is never refused where it is defined. The XMILE spec forbids the collision outright (v1.0 section 3.7.1 Arrays: dimension names "must be distinct from model variables names within the whole-model", docs/reference/xmile-v1.0.html), and several lowering sites assume the rule holds (the comment at Expr2::from's SIZE arm in src/simlin-engine/src/ast/expr2.rs says so explicitly: "we don't need to disambiguate between a dimension and variable with the same name - that's an invalid model per the spec. We check dimension names first"). Nothing enforces it, so every consumer that meets the ambiguous name resolves it on its own, and the same read is accepted or refused -- with a different code -- depending purely on the spelling that reaches it.
This breaks the invariant PR #1040's design doc states for Phase 7.5 (docs/design-plans/2026-08-25-compiler-unification.md, "helper == plain spelling"): a PREVIOUS/INIT helper reads what the plain spelling reads and is refused the way the plain spelling is refused. Here the plain spelling is accepted and the helper is refused, or the two are refused with different codes.
Reproduction
Minimal XMILE (files at the bottom); simlin simulate <file>. Dimension Other = {a, b}.
Arrayed collision -- <aux name="other"><dimensions><dim name="Other"/></dimensions><eqn>1</eqn></aux>, read by y:
compiles; y = [1, 2] -- the element index of dimension Other, not the variable's value [1, 1]
same
y[Other] = other[Other]
compiles; y = [1, 1] (reads the variable)
same
y[Other] = PREVIOUS(other)
NotSimulatable: "PREVIOUS requires a variable reference after helper rewriting" (codegen)
does_not_exist
y[Other] = INIT(other)
NotSimulatable: "INIT requires a variable reference argument" (codegen)
does_not_exist
y[Other] = PREVIOUS(other[Other])
NotSimulatable: "PREVIOUS requires a variable reference after helper rewriting"
same
y[Other] = INIT(other[Other])
NotSimulatable: "INIT requires a variable reference argument"
same
y = other[a], y = PREVIOUS(other[a])
compile
compile
y = SUM(other[*]), y = PREVIOUS(SUM(other[*]))
compile
compile
Scalar collision -- <aux name="other"><eqn>1</eqn></aux> (no dimensions):
y's equation
main
branch
define scalar other only
accepted, no diagnostic
same
y = other
dimension_in_scalar_context ("Dimension 'other' cannot be used in a scalar equation")
same
y = PREVIOUS(other) / y = INIT(other)
dimension_in_scalar_context
same
So for one model shape the outcomes range over: silent wrong numbers (bare read yields the element index), accepted (subscripted plain read), NotSimulatable from codegen with an internal-sounding message, DoesNotExist (branch), and DimensionInScalarContext -- and none of them points at the definition that is actually wrong. A release binary built from main on 2026-08-07 acceptedPREVIOUS(other[Other]) and INIT(other[Other]), so main's own answer has also drifted over time; nothing pins it.
Why it matters
Correctness: y[Other] = other with an arrayed variable other silently produces the dimension's element indices. Per the spec the model is invalid; the one acceptable outcome is a loud refusal, and this is the silent-wrong-number outcome.
Diagnostics consistency: the plain read and the PREVIOUS/INIT helper of the same read disagree, contradicting the Phase 7.5 invariant; the branch also changes one of the codes (NotSimulatable -> DoesNotExist for the bare captured spelling) without any test noticing, because no test enumerates the collision.
Developer/user experience: "PREVIOUS requires a variable reference after helper rewriting" describes compiler internals and blames the reference; dimension_in_scalar_context blames a reference to a variable the user defined. The actionable fact -- "other is both a variable and a dimension" -- is never said.
Where the decisions are made (all dimension-first, none guarded by a definition-time check)
src/simlin-engine/src/ast/expr3.rs, IndexExpr3::fromIndexExpr2::Expr arm: a bare name that is a dimension name becomes IndexExpr3::Dimension (element precedence, but not variable precedence).
src/simlin-engine/src/compiler/context.rs, the Var lowering: a dimension name in scalar context -> DimensionInScalarContext; in array context -> the element index.
src/simlin-engine/src/builtins_visitor.rs: index_spans_a_dimension, classify_snapshot_index, arg_needs_element_scope all ask "is this a dimension name" before "is this a variable", so PREVIOUS(other[Other]) is classified as a direct slot read whose base the compiler cannot then address, and codegen refuses it at src/simlin-engine/src/compiler/codegen.rs (static_slot returning None -> the two NotSimulatable messages above).
One definition-time refusal (preferred): next to model_duplicate_variables, derive the set of variables whose canonical name is also a canonical dimension name and emit one Error-severity diagnostic at the variable (a new code such as VariableShadowsDimension, or DuplicateVariable with details naming the dimension), and have compile_project_incremental refuse the model the way it refuses duplicate variables. Every downstream dimension-first rule then becomes sound by construction, and the per-spelling codes above never fire for this shape. The Expr2::from comment's assumption becomes a checked invariant.
If (1) is judged too strict for imported models (MDL corpora may carry the collision), at minimum route the capture path through the same refusal the plain spelling gets -- a parse/lowering-time code at the argument span -- instead of letting codegen refuse with NotSimulatable, and make the bare array-context read refuse rather than silently read the element index.
Whichever is chosen, the test should derive its rows from the spelling enumeration (definition only; bare in array context; subscripted; scalar context) x wrapper (plain; PREVIOUS; INIT; a module-function argument such as SMTH1) x variable shape (scalar; arrayed), asserting one code and one span for every row, on compile_project_incremental and on the CLI. Today no test covers any row of that table.
Discovery context
Out-of-scope discovery in the Phase 7.5c+d implementer report of PR #1040 (compiler-unification-v2); reproduced on origin/maind04593e6 and on the branch at d9de586a with fresh debug builds of simlin-cli. Pre-existing on main; the branch changes only the bare captured spelling's code (NotSimulatable -> DoesNotExist).
Files
Arrayed collision (plain, prev, init variants differ only in y's equation):
Summary
A variable whose canonical name equals a dimension's canonical name (
othervsOther) is never refused where it is defined. The XMILE spec forbids the collision outright (v1.0 section 3.7.1 Arrays: dimension names "must be distinct from model variables names within the whole-model",docs/reference/xmile-v1.0.html), and several lowering sites assume the rule holds (the comment atExpr2::from'sSIZEarm insrc/simlin-engine/src/ast/expr2.rssays so explicitly: "we don't need to disambiguate between a dimension and variable with the same name - that's an invalid model per the spec. We check dimension names first"). Nothing enforces it, so every consumer that meets the ambiguous name resolves it on its own, and the same read is accepted or refused -- with a different code -- depending purely on the spelling that reaches it.This breaks the invariant PR #1040's design doc states for Phase 7.5 (
docs/design-plans/2026-08-25-compiler-unification.md, "helper == plain spelling"): aPREVIOUS/INIThelper reads what the plain spelling reads and is refused the way the plain spelling is refused. Here the plain spelling is accepted and the helper is refused, or the two are refused with different codes.Reproduction
Minimal XMILE (files at the bottom);
simlin simulate <file>. DimensionOther = {a, b}.Arrayed collision --
<aux name="other"><dimensions><dim name="Other"/></dimensions><eqn>1</eqn></aux>, read byy:y's equationd04593e6compiler-unification-v2d9de586a(PR #1040)other[Other]onlyy[Other] = other(bare, array ctx)y = [1, 2]-- the element index of dimensionOther, not the variable's value[1, 1]y[Other] = other[Other]y = [1, 1](reads the variable)y[Other] = PREVIOUS(other)NotSimulatable: "PREVIOUS requires a variable reference after helper rewriting" (codegen)does_not_existy[Other] = INIT(other)NotSimulatable: "INIT requires a variable reference argument" (codegen)does_not_existy[Other] = PREVIOUS(other[Other])NotSimulatable: "PREVIOUS requires a variable reference after helper rewriting"y[Other] = INIT(other[Other])NotSimulatable: "INIT requires a variable reference argument"y = other[a],y = PREVIOUS(other[a])y = SUM(other[*]),y = PREVIOUS(SUM(other[*]))Scalar collision --
<aux name="other"><eqn>1</eqn></aux>(no dimensions):y's equationotheronlyy = otherdimension_in_scalar_context("Dimension 'other' cannot be used in a scalar equation")y = PREVIOUS(other)/y = INIT(other)dimension_in_scalar_contextSo for one model shape the outcomes range over: silent wrong numbers (bare read yields the element index), accepted (subscripted plain read),
NotSimulatablefrom codegen with an internal-sounding message,DoesNotExist(branch), andDimensionInScalarContext-- and none of them points at the definition that is actually wrong. A release binary built from main on 2026-08-07 acceptedPREVIOUS(other[Other])andINIT(other[Other]), so main's own answer has also drifted over time; nothing pins it.Why it matters
y[Other] = otherwith an arrayed variableothersilently produces the dimension's element indices. Per the spec the model is invalid; the one acceptable outcome is a loud refusal, and this is the silent-wrong-number outcome.PREVIOUS/INIThelper of the same read disagree, contradicting the Phase 7.5 invariant; the branch also changes one of the codes (NotSimulatable->DoesNotExistfor the bare captured spelling) without any test noticing, because no test enumerates the collision.dimension_in_scalar_contextblames a reference to a variable the user defined. The actionable fact -- "otheris both a variable and a dimension" -- is never said.Where the decisions are made (all dimension-first, none guarded by a definition-time check)
src/simlin-engine/src/ast/expr2.rs,Expr2::from,Expr1::Vararm:is_dimension_name && !is_array_context->DimensionInScalarContext.src/simlin-engine/src/ast/expr3.rs,IndexExpr3::fromIndexExpr2::Exprarm: a bare name that is a dimension name becomesIndexExpr3::Dimension(element precedence, but not variable precedence).src/simlin-engine/src/compiler/context.rs, theVarlowering: a dimension name in scalar context ->DimensionInScalarContext; in array context -> the element index.src/simlin-engine/src/builtins_visitor.rs:index_spans_a_dimension,classify_snapshot_index,arg_needs_element_scopeall ask "is this a dimension name" before "is this a variable", soPREVIOUS(other[Other])is classified as a direct slot read whose base the compiler cannot then address, and codegen refuses it atsrc/simlin-engine/src/compiler/codegen.rs(static_slotreturningNone-> the twoNotSimulatablemessages above).src/simlin-engine/src/db/diagnostic.rs,model_duplicate_variables/emit_duplicate_variable_diagnostics: the definition-time collision check that exists for variable-vs-variable (DuplicateVariable, GH engine: legacy ModelStage0 construction still silently collapses duplicate canonical variable idents (last-wins) #891) but has no variable-vs-dimension counterpart.Possible approaches
model_duplicate_variables, derive the set of variables whose canonical name is also a canonical dimension name and emit one Error-severity diagnostic at the variable (a new code such asVariableShadowsDimension, orDuplicateVariablewith details naming the dimension), and havecompile_project_incrementalrefuse the model the way it refuses duplicate variables. Every downstream dimension-first rule then becomes sound by construction, and the per-spelling codes above never fire for this shape. TheExpr2::fromcomment's assumption becomes a checked invariant.NotSimulatable, and make the bare array-context read refuse rather than silently read the element index.Whichever is chosen, the test should derive its rows from the spelling enumeration (definition only; bare in array context; subscripted; scalar context) x wrapper (plain;
PREVIOUS;INIT; a module-function argument such asSMTH1) x variable shape (scalar; arrayed), asserting one code and one span for every row, oncompile_project_incrementaland on the CLI. Today no test covers any row of that table.Discovery context
Out-of-scope discovery in the Phase 7.5c+d implementer report of PR #1040 (compiler-unification-v2); reproduced on
origin/maind04593e6and on the branch atd9de586awith fresh debug builds ofsimlin-cli. Pre-existing on main; the branch changes only the bare captured spelling's code (NotSimulatable->DoesNotExist).Files
Arrayed collision (
plain,prev,initvariants differ only iny's equation):Scalar collision: drop the
<dimensions>element fromotherand use<aux name="y"><eqn>PREVIOUS(other)</eqn></aux>.