runtime: null is not a dict, and a discarded interrogative is an error (#872, #869, #867) - #898
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens EigenScript’s runtime/compiler diagnostics by removing silent tolerance in two previously “absorbing” cases (null as a field/index receiver, and discarded interrogatives), and fixes a documented precedence pitfall in the language contract.
Changes:
- Runtime: make
null.field/null["k"]raise (while preserving dict-miss-returns-nullfor real dicts). - Compiler: emit a compile error when an interrogative statement is compiled in a discarded position (while keeping REPL/eval/expression-position interrogatives working).
- Docs: correct the midpoint-index idiom in
LANGUAGE_CONTRACT.mdand document the newnull-receiver access semantics; record changes inCHANGELOG.md.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_dict.eigs | Adds regression tests for null receiver field/index access raising and for chained access stopping at first null. |
| tests/run_all_tests.sh | Adds a new suite section asserting discarded interrogatives compile-error while REPL/eval paths remain live. |
| src/vm.c | Removes VAL_NULL exemptions from many field/index read sites so null no longer silently absorbs reads. |
| src/compiler.c | Adds a compile-stage check that flags discarded interrogative statements as compile errors. |
| src/lint.c | Switches interrogative word naming to a shared runtime table to avoid drift vs compiler. |
| src/eigenscript.c | Introduces eigs_interrogative_word() shared by lint and compiler. |
| src/eigenscript.h | Exposes eigs_interrogative_word() for cross-TU use. |
| docs/LANGUAGE_CONTRACT.md | Fixes midpoint-index example and documents null receiver access raising. |
| CHANGELOG.md | Notes the behavior changes and documentation fix in the changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
4143
to
4147
| } else if (target->type == VAL_DICT && idx->type == VAL_STR) { | ||
| dict_set(target, idx->data.str, val); | ||
| } else if (target->type != VAL_NULL) { | ||
| } else { | ||
| rt_error(EK_TYPE, current_line, "cannot index %s for assignment", val_type_name(target->type)); | ||
| } |
(#872, #869, #867) Three silent-tolerance items from the 2026-08 sweep. makes dict-miss-returns-null a deliberate decision (a missing key is a lookup miss, not a logic error), but that rationale covers a DICT. `null` is not a dict, and it was the one non-dict type on which field access silently succeeded — so a typo'd config path propagated through arbitrary depth (cfg.databse.host -> null -> null) and surfaced somewhere unrelated, or nowhere at all. Not a one-line special case: FIFTEEN field/index read sites in vm.c each carried an explicit `!= VAL_NULL` guard, so null was systematically absorbing for access. All fifteen are gone and the full suite passes unchanged — nothing in the tree depended on the absorption. A dict's own miss is still null, on purpose; what changed is that walking THROUGH a miss now fails at the miss. the literal 42, and had no effect at all: rc=0, nothing on stderr, only lint caught it. `what`, `when` and `where` are plausible variable names in exactly the domains this language targets. The issue proposed a parse-level compile error. That would have broken the REPL, which ECHOES `what is x` as `=> 5` — the two contexts are syntactically identical, and the only difference is whether the result is consumed. The compiler already knows that per statement, because compile_block and AST_PROGRAM emit OP_POP for every statement except the last. So the check keys on the DISCARD, not the syntax: the issue's repro is now a compile error, while the REPL's last statement, `eval`'s result, and interrogatives in expression position are untouched. No mode flag and no signature change. Honest limit, stated at the check: a discarded interrogative as a unit's FINAL statement is not caught here, because there it is the result. Lint's W019 still flags it. The interrogative word table moved into eigenscript.c so lint and the compiler cannot name different words for the same kind. binds tighter than `/` (per the precedence table 25 lines above in the same file), so `a[floor of (lo + hi) / 2]` parsed as `(floor of (lo + hi)) / 2` and hit the integer-index guard. Now `a[floor of ((lo + hi) / 2)]`, verified to run. tests/test_dict.eigs covers the null receiver on both access forms, the chained case, and that a dict miss is still null. New suite section [99j] covers the compile error plus the three contexts that must keep working (REPL echo, eval, expression position). Suite 3798/3798 release, 3796/3796 ASan+UBSan with detect_leaks=1, leak tally 0. Closes #872 Closes #869 Closes #867 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
InauguralPhysicist
force-pushed
the
fix/872-869-867-silent-tolerance
branch
from
August 5, 2026 22:59
1baee28 to
c749684
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/vm.c:1318
- This helper now raises for
nullon dot read (jit_helper_dot_get), but dot assignment still appears to tolerate aVAL_NULLreceiver (seejit_helper_dot_set’selse if (target->type != VAL_NULL)a few lines above this function). That leavesnull.k is vas a silent no-op even thoughnull.k/null["k"]are now loud; consider removing theVAL_NULLexemption in the DOT_SET paths for consistency with “null is not a dict”.
vm_push(v);
return;
}
} else {
rt_error(EK_TYPE, g_vm.current_line,
"cannot access field '%s' on %s",
key, val_type_name(target->type));
src/vm.c:4451
- In
CASE(LOCAL_IDX_DOT_SET), the non-dict list item guard still exemptsVAL_NULL(dict && dict->type != VAL_NULLa few lines above this branch). That meansxs[i].field is vcan still silently no-op whenxs[i]isnull, which is inconsistent with the new “null is not a dict” behavior for chained reads and index assignment.
rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)",
i, target->data.list.count);
}
} else if (target) {
rt_error(EK_TYPE, current_line, "cannot index %s for assignment",
val_type_name(target->type));
}
src/vm.c:4211
CASE(DOT_GET)now raises onVAL_NULL, butCASE(DOT_SET)still has atarget->type != VAL_NULLguard (silent no-op when assigning throughnull). Given the contract update in this PR (“null is not a dict”), dot assignment onnullshould likely raise too to prevent typo’d config paths from silently discarding writes.
vm_push(v);
DISPATCH();
}
} else {
rt_error(EK_TYPE, current_line, "cannot access field '%s' on %s",
key, val_type_name(target->type));
}
tests/run_all_tests.sh:4321
- The new [99j] check only asserts the first output line of
live.eigsis3, so a regression in the expression-position interrogative (print of (str of (what is z))) could slip by as long asevalstill works. Checking both printed lines will make this test actually cover both required contexts.
# Same for `eval`, and for an interrogative used inside an expression.
printf 'z is 3\nprint of (str of (eval of "what is z"))\nprint of (str of (what is z))\n' > "$SK_DIR/live.eigs"
SK_LIVE=$(./eigenscript "$SK_DIR/live.eigs" 2>&1); SK_LIVE_RC=$?
TOTAL=$((TOTAL + 1))
if [ "$SK_LIVE_RC" -eq 0 ] && [ "$(echo "$SK_LIVE" | head -1)" = "3" ]; then
PASS=$((PASS + 1))
echo " PASS: eval and expression-position interrogatives are untouched"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three silent-tolerance items from the 2026-08 sweep.
#872 —
nullwas the one non-dict you could read fields offThe contract makes dict-miss-returns-
nulla deliberate decision: a missing key is a lookup miss, not a logic error. That rationale covers a dict.nullis not a dict, and it was the one non-dict type on which field access silently succeeded — so a typo'd config path propagated through arbitrary depth (cfg.databse.host→null→null) and surfaced somewhere unrelated, or nowhere.Not a one-line special case. Fifteen field/index read sites in
vm.ceach carried an explicit!= VAL_NULLguard — null was systematically absorbing for access. All fifteen are gone, and the full suite passes unchanged, which is the useful finding: nothing in the tree depended on the absorption.A dict's own miss is still
null, on purpose. What changed is that walking through a miss fails at the miss:#869 — the issue's proposed fix would have broken the REPL
what is 42reads as an assignment, parses as a question about the literal42, and had no effect at all: rc=0, nothing on stderr, only--lintcaught it.what,whenandwhereare plausible variable names in exactly the domains this language targets.The issue proposed making soft-keyword assignment a parse-level compile error. I checked the REPL first — it echoes
what is xas=> 5. A parse-level error would have broken the language's most natural interrogative use, andeval of "what is x"with it. The two contexts are syntactically identical; the only difference is whether the result is consumed.The compiler already knows that per statement:
compile_blockandAST_PROGRAMemitOP_POPfor every statement except the last. So the check keys on the discard, not the syntax.while all three live contexts are untouched:
No mode flag, no signature change. Honest limit, stated at the check: a discarded interrogative as a unit's final statement isn't caught here, because there it is the result — lint's W019 still flags it.
The interrogative word table moved into
eigenscript.cso lint and the compiler cannot name different words for the same kind.#867 — the documented midpoint index raised
ofbinds tighter than/(per the precedence table 25 lines above in the same file), so the recommendeda[floor of (lo + hi) / 2]parsed as(floor of (lo + hi)) / 2and hit the integer-index guard. Nowa[floor of ((lo + hi) / 2)], verified to run.Verification
tests/test_dict.eigscovers the null receiver on both access forms, the chained case, and that a dict miss is stillnull. New suite section [99j] covers the compile error plus the three contexts that must keep working.detect_leaks=1: 3796/3796, leak tally 0Closes #872
Closes #869
Closes #867
🤖 Generated with Claude Code