Stop an armed TRAP from swallowing parse errors whole
A program with TRAP set and a line that would not parse printed nothing at all and exited zero. No error line, because akbasic_runtime_error() intercepts for the trap and deliberately prints nothing; and no handler, because the parse branch of process_line_run() then set run_finished_mode regardless -- QUIT for a file -- so the next line boundary the handler would have been entered at never came. Arming an error handler made errors disappear. The guard is to set the finished mode only when the error was actually reported. The runtime path was always right: report_and_reraise() never touched the mode. The same branch also never recorded the status, so the handler that now runs was handed ER# 0. It sets lasterrorstatus from the context the way report_and_reraise() does, and a trapped SCALE with no arguments reports 512. Found while writing the error-code appendix -- documenting what ER# can hold means provoking each code, and this is what happened on the way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
20
TODO.md
20
TODO.md
@@ -1592,11 +1592,27 @@ fix is reverted:
|
|||||||
`PASS`. A `VERIFY` against a file that did not match, which is an ordinary user answer rather
|
`PASS`. A `VERIFY` against a file that did not match, which is an ordinary user answer rather
|
||||||
than a fault, came out as a stack trace and would have taken an embedding host with it.
|
than a fault, came out as a stack trace and would have taken an embedding host with it.
|
||||||
`tests/housekeeping_verbs.c`.
|
`tests/housekeeping_verbs.c`.
|
||||||
|
3. **An armed `TRAP` made a parse error vanish outright.** Found the same way, writing §8
|
||||||
|
item 11's error-code appendix: a program with `TRAP` set and a line that would not parse
|
||||||
|
printed *nothing at all* and exited zero. No error line, because
|
||||||
|
`akbasic_runtime_error()` intercepts for the trap and deliberately prints nothing; and no
|
||||||
|
handler, because the parse branch of `process_line_run()` then set `run_finished_mode`
|
||||||
|
regardless — QUIT for a file run — so the next line boundary the handler would have been
|
||||||
|
entered at was never reached. **Arming an error handler made errors disappear**, which is
|
||||||
|
the exact opposite of the verb's purpose, and it was silent in both directions.
|
||||||
|
|
||||||
Neither was on any list. Both were found by writing down what a chapter claimed and then
|
The guard is one `if`: set the finished mode only when the error was actually reported.
|
||||||
|
The runtime path was always right, because `report_and_reraise()` never touched the mode.
|
||||||
|
|
||||||
|
**And the same branch never recorded the status**, so the handler that now runs was
|
||||||
|
handed `ER# 0`. It sets `obj->lasterrorstatus` from the context the way
|
||||||
|
`report_and_reraise()` does, and a trapped `SCALE` with no arguments now reports 512.
|
||||||
|
Both halves in `tests/trap_verbs.c`.
|
||||||
|
|
||||||
|
None of the three was on any list. All were found by writing down what a chapter claimed and then
|
||||||
running it — which is the argument for the harness rather than a footnote to it.
|
running it — which is the argument for the harness rather than a footnote to it.
|
||||||
|
|
||||||
**A third is open, and it is the same defect as item 2 on a path that fix did not cover.**
|
**A fourth is open, and it is the same defect as item 2 on a path that fix did not cover.**
|
||||||
Writing `docs/14-architecture.md` meant describing where the boundary between a script's
|
Writing `docs/14-architecture.md` meant describing where the boundary between a script's
|
||||||
error and the host's error actually sits, and the answer is: around parsing and around
|
error and the host's error actually sits, and the answer is: around parsing and around
|
||||||
`interpret()`, but **not around scanning**. `akbasic_runtime_process_line_repl()` and
|
`interpret()`, but **not around scanning**. `akbasic_runtime_process_line_repl()` and
|
||||||
|
|||||||
@@ -834,6 +834,7 @@ akerr_ErrorContext *akbasic_runtime_process_line_repl(akbasic_Runtime *obj)
|
|||||||
} HANDLE_DEFAULT(errctx) {
|
} HANDLE_DEFAULT(errctx) {
|
||||||
char message[AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH];
|
char message[AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH];
|
||||||
snprintf(message, sizeof(message), "%s", errctx->message);
|
snprintf(message, sizeof(message), "%s", errctx->message);
|
||||||
|
obj->lasterrorstatus = errctx->status;
|
||||||
IGNORE(akbasic_runtime_error(obj, AKBASIC_ERRCLASS_PARSE, message));
|
IGNORE(akbasic_runtime_error(obj, AKBASIC_ERRCLASS_PARSE, message));
|
||||||
} FINISH(errctx, false);
|
} FINISH(errctx, false);
|
||||||
if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) {
|
if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) {
|
||||||
@@ -930,8 +931,26 @@ akerr_ErrorContext *akbasic_runtime_process_line_run(akbasic_Runtime *obj)
|
|||||||
} HANDLE_DEFAULT(errctx) {
|
} HANDLE_DEFAULT(errctx) {
|
||||||
char message[AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH];
|
char message[AKERR_MAX_ERROR_CONTEXT_STRING_LENGTH];
|
||||||
snprintf(message, sizeof(message), "%s", errctx->message);
|
snprintf(message, sizeof(message), "%s", errctx->message);
|
||||||
|
/*
|
||||||
|
* What ER# reports, recorded before the context goes -- the same line
|
||||||
|
* report_and_reraise() carries for a runtime error, and it was missing
|
||||||
|
* here, so a trapped parse error handed the handler ER# 0.
|
||||||
|
*/
|
||||||
|
obj->lasterrorstatus = errctx->status;
|
||||||
IGNORE(akbasic_runtime_error(obj, AKBASIC_ERRCLASS_PARSE, message));
|
IGNORE(akbasic_runtime_error(obj, AKBASIC_ERRCLASS_PARSE, message));
|
||||||
IGNORE(akbasic_runtime_set_mode(obj, obj->run_finished_mode));
|
/*
|
||||||
|
* Only when the error was actually reported. An armed TRAP intercepts
|
||||||
|
* inside akbasic_runtime_error() and leaves `errclass` clear, meaning
|
||||||
|
* "the handler runs at the next line boundary" -- and ending the run
|
||||||
|
* here reaches that boundary never. Setting the mode unconditionally
|
||||||
|
* made a parse error under a TRAP vanish outright: no error line,
|
||||||
|
* because the trap suppressed it, and no handler, because
|
||||||
|
* run_finished_mode for a file is QUIT. Arming an error handler made
|
||||||
|
* errors disappear, which is the opposite of what it is for.
|
||||||
|
*/
|
||||||
|
if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) {
|
||||||
|
IGNORE(akbasic_runtime_set_mode(obj, obj->run_finished_mode));
|
||||||
|
}
|
||||||
} FINISH(errctx, false);
|
} FINISH(errctx, false);
|
||||||
if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) {
|
if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) {
|
||||||
SUCCEED_RETURN(errctx);
|
SUCCEED_RETURN(errctx);
|
||||||
|
|||||||
@@ -53,6 +53,47 @@ static void test_trap_catches(void)
|
|||||||
harness_stop();
|
harness_stop();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief A *parse* error is trappable too, and carries its code into ER#.
|
||||||
|
*
|
||||||
|
* Found while writing the error-code appendix, and it was the worst kind of
|
||||||
|
* defect: arming a handler made errors disappear. `akbasic_runtime_error()`
|
||||||
|
* intercepts for the trap and leaves `errclass` clear, but the parse branch of
|
||||||
|
* `process_line_run()` set `run_finished_mode` regardless -- QUIT for a file --
|
||||||
|
* so the run ended before the next line boundary the handler would have been
|
||||||
|
* entered at. No error line, because the trap suppressed it; no handler, because
|
||||||
|
* the program had stopped. Nothing on stdout at all.
|
||||||
|
*
|
||||||
|
* The second half is that the same branch never recorded the status, so the
|
||||||
|
* handler that now runs used to be handed `ER# 0`.
|
||||||
|
*/
|
||||||
|
static void test_trap_catches_parse_error(void)
|
||||||
|
{
|
||||||
|
TEST_REQUIRE_OK(run_program("10 TRAP 100\n"
|
||||||
|
"20 SCALE\n"
|
||||||
|
"30 PRINT \"AFTER\"\n"
|
||||||
|
"40 END\n"
|
||||||
|
"100 PRINT \"TRAPPED \" + ER# + \" ON LINE \" + EL#\n"
|
||||||
|
"110 RESUME NEXT\n"));
|
||||||
|
/* 512 is AKBASIC_ERR_SYNTAX, the base of the band. Spelled out for the same
|
||||||
|
reason test_trap_catches() spells out 515. */
|
||||||
|
TEST_REQUIRE_STR(HARNESS_OUTPUT, "TRAPPED 512 ON LINE 20\nAFTER\n");
|
||||||
|
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "PARSE ERROR") == NULL,
|
||||||
|
"a trapped parse error must not also be reported, got \"%s\"",
|
||||||
|
HARNESS_OUTPUT);
|
||||||
|
harness_stop();
|
||||||
|
|
||||||
|
/* And with no trap it still reports and still stops -- the half of the
|
||||||
|
behaviour that was never broken and must not become so. */
|
||||||
|
TEST_REQUIRE_OK(run_program("10 SCALE\n"
|
||||||
|
"20 PRINT \"AFTER\"\n"));
|
||||||
|
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "? 10 : PARSE ERROR") != NULL,
|
||||||
|
"an untrapped parse error should be reported, got \"%s\"", HARNESS_OUTPUT);
|
||||||
|
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "AFTER") == NULL,
|
||||||
|
"an untrapped parse error should stop the run, got \"%s\"", HARNESS_OUTPUT);
|
||||||
|
harness_stop();
|
||||||
|
}
|
||||||
|
|
||||||
/** @brief With no TRAP the error is reported and the program stops, as before. */
|
/** @brief With no TRAP the error is reported and the program stops, as before. */
|
||||||
static void test_untrapped_still_reports(void)
|
static void test_untrapped_still_reports(void)
|
||||||
{
|
{
|
||||||
@@ -184,6 +225,7 @@ static void test_err_function(void)
|
|||||||
int main(void)
|
int main(void)
|
||||||
{
|
{
|
||||||
test_trap_catches();
|
test_trap_catches();
|
||||||
|
test_trap_catches_parse_error();
|
||||||
test_untrapped_still_reports();
|
test_untrapped_still_reports();
|
||||||
test_trap_disarms();
|
test_trap_disarms();
|
||||||
test_trap_by_label();
|
test_trap_by_label();
|
||||||
|
|||||||
Reference in New Issue
Block a user