Preserve runtime errors over the statement-terminator check#79
Open
Bascy wants to merge 1 commit into
Open
Conversation
Previously, when a runtime error was returned from a native C function in the middle of an expression (e.g. `myCFunc(badArg) * 60;`), the engine would clobber the original error message with a misleading "; expected" parse error. Root cause: 1. The native function calls `js_mkerr(...)` which sets `js->tok = TOK_EOF` and `js->pos = js->clen` to short-circuit further parsing. 2. `do_call_op` restores the parser state after the call returns (so the parser is sitting on the `*` token after the function call). 3. The `RTL_BINOP` / `LTR_BINOP` loops short-circuit on `is_err(res)` and do NOT consume the trailing `*` operator. 4. `js_stmt` then reaches its terminator check `next(js) != TOK_SEMICOLON && next(js) != TOK_EOF && next(js) != TOK_RBRACE` without gating on `is_err(res)`, sees the unconsumed `*`, and overwrites the real error with "; expected" (errmsg is a single shared buffer). Fix: short-circuit in `js_stmt` when `res` is already an error, so the original error message is preserved. Tests added to cover this case for C-function-returned errors embedded in larger expressions.
c514068 to
3d729ea
Compare
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.
Summary
When a native (C) function returns an error mid-expression (e.g.
myCFunc(badArg) * 60;), the engine currently reports"; expected"instead of the original error message.Reproducing
Root cause
js_mkerr(...)which setsjs->tok = TOK_EOFandjs->pos = js->clento short-circuit further parsing.do_call_oprestores the parser state after the call returns (so the parser is sitting on the*token after)).RTL_BINOP/LTR_BINOPshort-circuit onis_err(res)and do not consume the trailing operator.js_stmt's terminator checkis_err(res), sees the unconsumed*, and overwrites the real error (errmsgis a single shared buffer).Fix
Short-circuit in
js_stmtwhenresis already an error. One-line change inelk.c; existing terminator behavior for non-error statements is unchanged.Test plan
test_c_funcscovering C-function errors embedded in larger expressions (gt() * 2,1 + gt(null, 1))."; expected"(e.g."1 2","1 + 2 3","{1}",function(){1}) still pass — they exercise expression-level non-error results followed by stray tokens, which is unaffected by this patch.