andrew/akbasic
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refuse a branch to a line the program did not number

Browse Source 
GOTO 100 in a script written without line numbers finds the hundredth line
and branches there. Silent, plausible and wrong: the test for it loops
forever printing the second line when the check is removed.

akbasic_runtime_check_targets() is a fourth prescan beside the label, DATA
and TYPE ones, run on every entry into MODE_RUN -- the earliest the check
can be made and the only place all four ways a program arrives pass
through. A target naming an empty line is still allowed, for the same
reason RENUMBER leaves one alone, and a fully numbered program is
unaffected, which is every program that existed before this.

It shares RENUMBER's walk rather than repeating it. renumber.c grows an
akbasic_TargetWalk -- a self pointer and a visit function -- and
rewrite_line() takes one. RENUMBER's visitor substitutes the number a line
moved to; the check's substitutes the number unchanged and raises. One
walk, so the two cannot disagree about what a branch target is.

The check points environment->lineno at the line being walked so the "? N :"
prefix names the offending line. The other three prescans do not and report
whichever line the loader stopped on; TODO.md section 5 item 64 records it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andrew Kesterson
2026-08-01 16:38:54 -04:00
parent 8bc253ebbf
commit 6273be580a
7 changed files with 341 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
TODO.md
View File

@@ -1385,6 +1385,40 @@ deviations from the reference's *program*: `main.go` and the SDL half of
the last, as they always have. That is a separate decision with its own corpus risk and it
is not this one.
64. **Branching by number to a line the program did not number is refused before it runs.**
The other half of deviation 63, and the reason `akbasic_SourceLine` carries a flag rather
than the loader just picking slots quietly. In a script written without line numbers
`GOTO 100` finds the hundredth line and branches there: plausible, silent and wrong, which
is the worst thing to hand somebody at run time.
`akbasic_runtime_check_targets()` is a fourth prescan, run beside the label, `DATA` and
`TYPE` ones on every entry into `AKBASIC_MODE_RUN` — the earliest the check can be made and
the only place all four ways a program arrives pass through. It refuses with
`AKBASIC_ERR_SYNTAX`, naming the line and saying what to do instead:
```text
? 1 : PARSE ERROR Line 1: branch to line 4, which the program did not number.
Branch by LABEL, or RENUMBER first
```
Two things are deliberately **not** refused. A target naming an *empty* line, for the same
reason `RENUMBER` leaves one alone — `GOTO 9999` in a program with no line 9999 is already
broken and inventing a rule about it would hide that. And a target in a fully numbered
program, which is every program that existed before this, so nothing checked in changed.
**It shares `RENUMBER`'s walk rather than repeating it.** `src/renumber.c` grew an
`akbasic_TargetWalk` — a `self` pointer and a `visit` function — and `rewrite_line()` now
takes one. `RENUMBER`'s visitor substitutes the number a line moved to; the check's
substitutes the number unchanged and raises if it names an assigned line. One walk, so the
two cannot disagree about what a branch target is, which they would have within a release.
Worth knowing: the check points `environment->lineno` at the line it is walking, so the
`? N :` prefix names the offending line. **The other three prescans do not**, so a
malformed `TYPE` or a bad `DATA` item still reports whichever line the loader stopped on —
usually the last line of the program. They work around it by putting `Line %d:` in the
message text, which is why the message above says the number twice. Worth fixing in
`akbasic_runtime_set_mode()`'s wrapper rather than in four places; not fixed here.
---
---

23
include/akbasic/runtime.h
View File

@@ -388,6 +388,29 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_settime(akbasic_Runtime *obj,
*/
akerr_ErrorContext AKERR_NOIGNORE *akbasic_renumber(akbasic_Runtime *obj, int64_t newstart, int64_t increment, int64_t oldstart);
/**
* @brief Refuse a program that branches by number to a line it did not number.
*
* The fourth prescan, run beside the label, `DATA` and `TYPE` ones on every entry
* into AKBASIC_MODE_RUN. Since a loaded line may be given its number rather than
* carry one, `GOTO 100` in a script written without numbers finds the hundredth
* line and branches there -- plausible, silent and wrong. This says so before any
* line executes, which is the earliest it can be said and the only place that
* covers all four ways a program arrives.
*
* It walks the same `GOTO`, `GOSUB`, `RUN`, `RESTORE`, `TRAP`, `ON ... GOTO` and
* `COLLISION` targets akbasic_renumber() rewrites, sharing that walk rather than
* repeating it. A target naming an *empty* line is allowed through, for the same
* reason RENUMBER leaves one alone. A `GOTO` to a label never reaches here.
*
* @param obj Object to initialize, inspect, or modify.
* @return `NULL` on success, otherwise an error context owned by the caller.
* @throws AKERR_NULLPOINTER When `obj` is NULL.
* @throws AKBASIC_ERR_SYNTAX When a numeric target names a line the program did not number.
* @throws AKBASIC_ERR_BOUNDS When a line does not fit the walk's buffer.
*/
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_check_targets(akbasic_Runtime *obj);
/**
* @brief File every `LABEL` in the stored program before any of it runs.
*

161
src/renumber.c
View File

@@ -1,18 +1,26 @@
/**
* @file renumber.c
* @brief RENUMBER: move every line, and rewrite every reference to one.
* @brief Walking a program's branch targets, and the two things that do it.
*
* Moving the lines is easy -- source is filed under its line number in
* `source[]`, so it is a permutation. Rewriting every `GOTO`, `GOSUB`, `RUN`,
* `RESTORE`, `TRAP` and `COLLISION` target to match is the job, and it is why
* this was deferred rather than written early: a `RENUMBER` that moved lines
* without fixing their references would silently break every branch in the
* program, which is worse than not having the verb at all.
* **RENUMBER** moves every line and rewrites every reference to one. Moving the
* lines is easy -- source is filed under its line number in `source[]`, so it is
* a permutation. Rewriting every `GOTO`, `GOSUB`, `RUN`, `RESTORE`, `TRAP` and
* `COLLISION` target to match is the job, and it is why this was deferred rather
* than written early: a `RENUMBER` that moved lines without fixing their
* references would silently break every branch in the program, which is worse
* than not having the verb at all.
*
* The rewrite is textual, and the only thing it has to understand about the rest
* of the language is where a string literal starts and stops -- `PRINT "GOTO 10"`
* **akbasic_runtime_check_targets()** walks the same targets without moving
* anything, refusing a program that branches by number to a line the program
* never numbered. It shares this file because it needs exactly the same walk,
* and a second copy of it would drift.
*
* The walk is textual, and the only thing it has to understand about the rest of
* the language is where a string literal starts and stops -- `PRINT "GOTO 10"`
* must come through untouched. That is the same statement-walking the label and
* DATA prescans do.
* DATA prescans do. What the two callers differ in is one function pointer:
* given a target, say what text should stand in its place, and raise if it is not
* acceptable.
*/
#include <ctype.h>
@@ -43,6 +51,21 @@ static const char *BRANCH_VERBS[] = { "GOTO", "GOSUB", "RUN", "RESTORE", "TRAP"
/** @brief How many entries #BRANCH_VERBS has. */
#define BRANCH_VERB_COUNT ((int)(sizeof(BRANCH_VERBS) / sizeof(BRANCH_VERBS[0])))
/**
* @brief What a walk does with each numeric branch target it finds.
*
* The one thing RENUMBER and the target check differ in. `visit` is handed a
* target and writes into @p dest the text that should stand in its place --
* the new number for RENUMBER, the same number for a check -- and may raise to
* refuse the line outright.
*/
typedef struct akbasic_TargetWalk akbasic_TargetWalk;
struct akbasic_TargetWalk
{
void *self;
akerr_ErrorContext *(*visit)(akbasic_TargetWalk *walk, int64_t target, char *dest, size_t len);
};
/**
* @brief Where a line moved to, or the line itself when it did not move.
*
@@ -59,25 +82,26 @@ static int64_t mapped(const int64_t *map, int64_t line)
}
/**
* @brief Copy a run of comma-separated line numbers, rewriting each.
* @brief Copy a run of comma-separated line numbers, visiting each.
*
* `ON X GOTO 100, 200, 300` is why this takes a list rather than one number: the
* targets follow the `GOTO`, so handling `GOTO` handles `ON` for free.
*
* @param map Old line to new line.
* @param walk What to substitute for each target, and whether to allow it.
* @param src Reads from here; advanced past what was consumed.
* @param dest Writes to here; advanced past what was written.
* @param limit One past the last byte @p dest may write.
* @return `NULL` on success, otherwise an error context owned by the caller.
* @throws AKBASIC_ERR_BOUNDS When the rewritten line does not fit.
*/
static akerr_ErrorContext *rewrite_targets(const int64_t *map, const char **src, char **dest, const char *limit)
static akerr_ErrorContext *rewrite_targets(akbasic_TargetWalk *walk, const char **src, char **dest, const char *limit)
{
PREPARE_ERROR(errctx);
const char *p = *src;
char *out = *dest;
for ( ;; ) {
char replacement[32];
int64_t line = 0;
int written = 0;
@@ -101,7 +125,8 @@ static akerr_ErrorContext *rewrite_targets(const int64_t *map, const char **src,
line = (line * 10) + (*p - '0');
p += 1;
}
written = snprintf(out, (size_t)(limit - out), "%" PRId64, mapped(map, line));
PASS(errctx, walk->visit(walk, line, replacement, sizeof(replacement)));
written = snprintf(out, (size_t)(limit - out), "%s", replacement);
FAIL_ZERO_RETURN(errctx, (written > 0 && out + written < limit), AKBASIC_ERR_BOUNDS,
"RENUMBER: a rewritten line does not fit");
out += written;
@@ -136,14 +161,14 @@ static akerr_ErrorContext *rewrite_targets(const int64_t *map, const char **src,
/**
* @brief Rewrite every branch target in one line of source.
*
* @param map Old line to new line.
* @param walk What to substitute for each target, and whether to allow it.
* @param code The line to read.
* @param dest Where the rewritten line goes.
* @param len Capacity of @p dest.
* @return `NULL` on success, otherwise an error context owned by the caller.
* @throws AKBASIC_ERR_BOUNDS When the rewritten line does not fit.
*/
static akerr_ErrorContext *rewrite_line(const int64_t *map, const char *code, char *dest, size_t len)
static akerr_ErrorContext *rewrite_line(akbasic_TargetWalk *walk, const char *code, char *dest, size_t len)
{
PREPARE_ERROR(errctx);
const char *p = code;
@@ -210,7 +235,7 @@ static akerr_ErrorContext *rewrite_line(const int64_t *map, const char *code, ch
memcpy(out, p, verblen);
out += verblen;
p += verblen;
PASS(errctx, rewrite_targets(map, &p, &out, limit));
PASS(errctx, rewrite_targets(walk, &p, &out, limit));
statementstart = false;
continue;
}
@@ -235,7 +260,7 @@ static akerr_ErrorContext *rewrite_line(const int64_t *map, const char *code, ch
*out = ',';
out += 1;
p += 1;
PASS(errctx, rewrite_targets(map, &p, &out, limit));
PASS(errctx, rewrite_targets(walk, &p, &out, limit));
}
statementstart = false;
continue;
@@ -251,11 +276,24 @@ static akerr_ErrorContext *rewrite_line(const int64_t *map, const char *code, ch
SUCCEED_RETURN(errctx);
}
/* ----------------------------------------------------------- RENUMBER -- */
/** @brief RENUMBER's visitor: substitute the number the line moved to. */
static akerr_ErrorContext *visit_renumber(akbasic_TargetWalk *walk, int64_t target, char *dest, size_t len)
{
PREPARE_ERROR(errctx);
const int64_t *map = (const int64_t *)walk->self;
snprintf(dest, len, "%" PRId64, mapped(map, target));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akbasic_renumber(akbasic_Runtime *obj, int64_t newstart, int64_t increment, int64_t oldstart)
{
PREPARE_ERROR(errctx);
static int64_t map[AKBASIC_MAX_SOURCE_LINES];
static akbasic_SourceLine rewritten[AKBASIC_MAX_SOURCE_LINES];
akbasic_TargetWalk walk = { map, visit_renumber };
int64_t next = newstart;
int64_t i = 0;
@@ -305,7 +343,7 @@ akerr_ErrorContext *akbasic_renumber(akbasic_Runtime *obj, int64_t newstart, int
* `oldstart` can branch into the region that moved.
*/
target = mapped(map, i);
PASS(errctx, rewrite_line(map, obj->source[i].code,
PASS(errctx, rewrite_line(&walk, obj->source[i].code,
rewritten[target].code, sizeof(rewritten[target].code)));
rewritten[target].lineno = target;
/*
@@ -320,3 +358,88 @@ akerr_ErrorContext *akbasic_renumber(akbasic_Runtime *obj, int64_t newstart, int
memcpy(obj->source, rewritten, sizeof(obj->source));
SUCCEED_RETURN(errctx);
}
/* ------------------------------------------------- the target prescan -- */
/** @brief What visit_check() needs to name the line it refused. */
typedef struct
{
const akbasic_Runtime *runtime;
int64_t lineno;
} CheckState;
/**
* @brief The prescan's visitor: allow the target, or refuse the program.
*
* A number that names a line the program never numbered is refused. In a script
* written without line numbers `GOTO 100` finds the hundredth line and branches
* there -- plausible, silent, and wrong, which is the worst kind of defect to be
* handed at run time.
*
* A target naming an *empty* slot is allowed through, for the same reason
* mapped() leaves one alone: `GOTO 9999` in a program with no line 9999 is
* already broken, and refusing it here would be inventing a rule about lines
* that do not exist. The substitution is the number itself, because nothing is
* being rewritten.
*/
static akerr_ErrorContext *visit_check(akbasic_TargetWalk *walk, int64_t target, char *dest, size_t len)
{
PREPARE_ERROR(errctx);
CheckState *state = (CheckState *)walk->self;
snprintf(dest, len, "%" PRId64, target);
if ( target >= 0 && target < AKBASIC_MAX_SOURCE_LINES ) {
FAIL_NONZERO_RETURN(errctx,
(state->runtime->source[target].code[0] != '\0'
&& !state->runtime->source[target].numbered),
AKBASIC_ERR_SYNTAX,
"Line %" PRId64 ": branch to line %" PRId64
", which the program did not number. Branch by LABEL, or RENUMBER first",
state->lineno, target);
}
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akbasic_runtime_check_targets(akbasic_Runtime *obj)
{
PREPARE_ERROR(errctx);
/*
* Static for the same reason akbasic_renumber()'s buffers are: a line's worth
* of rewrite does not belong on a default stack alongside the rest of a
* step(). Nothing is read back out of it -- the walk needs somewhere to put
* the text it would have written, and this is it.
*/
static char discard[AKBASIC_MAX_LINE_LENGTH * 2];
CheckState state = { NULL, 0 };
akbasic_TargetWalk walk = { &state, visit_check };
int64_t entry = 0;
int64_t i = 0;
FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL runtime in check_targets");
state.runtime = obj;
entry = obj->environment->lineno;
for ( i = 0; i < AKBASIC_MAX_SOURCE_LINES; i++ ) {
if ( obj->source[i].code[0] == '\0' ) {
continue;
}
state.lineno = i;
/*
* Point the runtime at the line being walked, so the `? N :` prefix
* akbasic_runtime_error() builds from environment->lineno names the
* offending line rather than whichever one the loader stopped on. The
* message repeats the number because the other three prescans do not do
* this and get it wrong -- see TODO.md.
*
* PASS inside the loop, never CATCH: CATCH expands to a break, which
* would leave this loop rather than an enclosing ATTEMPT. That also means
* the cursor is deliberately *not* restored on the failure path, which is
* the whole point of setting it.
*/
obj->environment->lineno = i;
PASS(errctx, rewrite_line(&walk, obj->source[i].code, discard, sizeof(discard)));
}
/* Nothing was refused, so leave the cursor as the caller had it. */
obj->environment->lineno = entry;
SUCCEED_RETURN(errctx);
}

12
src/runtime.c
View File

@@ -380,7 +380,7 @@ akerr_ErrorContext *akbasic_runtime_set_mode(akbasic_Runtime *obj, int mode)
*/
if ( obj->mode == AKBASIC_MODE_RUN && obj->environment != NULL ) {
/*
* All three prescans, and all three inside one ATTEMPT.
* All four prescans, and all four inside one ATTEMPT.
*
* **A prescan failure is the program's mistake, not the host's**, so it
* has to leave here as a BASIC error line rather than as a raised
@@ -417,6 +417,16 @@ akerr_ErrorContext *akbasic_runtime_set_mode(akbasic_Runtime *obj, int mode)
* skipped the lines that declared it.
*/
CATCH(errctx, akbasic_structtype_scan(obj));
/*
* Then the branch targets, last because it is the only one that reads
* nothing into the runtime -- it only refuses. A loaded line may be
* *given* its number rather than carry one, so `GOTO 100` in a script
* written without numbers would otherwise find the hundredth line and
* branch there, which is plausible, silent and wrong. Said here rather
* than at the branch, because before the program runs is earlier and
* every path into a run comes through this function.
*/
CATCH(errctx, akbasic_runtime_check_targets(obj));
} CLEANUP {
} PROCESS(errctx) {
} HANDLE_DEFAULT(errctx) {

20
tests/language/statements/unnumbered.bas Normal file
View File

@@ -0,0 +1,20 @@
REM A whole program with no line numbers at all, read through RUNSTREAM.
REM Blank lines cost nothing, and every branch is by LABEL.
PRINT "START"
FOR I# = 1 TO 3
PRINT I# * I#
NEXT I#
GOSUB GREET
GOTO FINISH
PRINT "NOT REACHED"
LABEL GREET
PRINT "HELLO"
RETURN
LABEL FINISH
PRINT "END"

6
tests/language/statements/unnumbered.txt Normal file
View File

@@ -0,0 +1,6 @@
START
1
4
9
HELLO
END

105
tests/unnumbered.c
View File

@@ -217,6 +217,105 @@ static void test_prompt_still_runs_direct_mode(void)
harness_stop();
}
/**
* @brief Branching by number to a line nobody numbered is refused before the run.
*
* `GOTO 2` in a program with no line numbers finds the second line and branches
* there. The prescan says so instead, and says it before any line executes -- so
* the program produces the error and nothing else, not the error partway through
* its output.
*/
static void test_numeric_branch_to_an_assigned_line_is_refused(void)
{
TEST_REQUIRE_OK(run_program("PRINT \"A\"\n"
"PRINT \"B\"\n"
"GOTO 2\n"));
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "did not number") != NULL,
"expected a refusal naming the unnumbered target, got \"%s\"", HARNESS_OUTPUT);
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "A\n") == NULL,
"the program should not have run at all, got \"%s\"", HARNESS_OUTPUT);
harness_stop();
/* The refusal names the offending line, not wherever the loader stopped. */
TEST_REQUIRE_OK(run_program("GOTO 4\n"
"PRINT \"B\"\n"
"PRINT \"C\"\n"
"PRINT \"D\"\n"
"PRINT \"E\"\n"));
TEST_REQUIRE(strstr(HARNESS_OUTPUT, "? 1 :") != NULL,
"expected the error to name line 1, got \"%s\"", HARNESS_OUTPUT);
harness_stop();
}
/** @brief A written number is still a legal target, in a mixed program too. */
static void test_numeric_branch_to_a_written_line_is_allowed(void)
{
TEST_REQUIRE_OK(run_program("10 PRINT \"A\"\n"
"20 IF A# = 0 THEN A# = 1 : GOTO 10\n"
"30 PRINT \"OK\"\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "A\nA\nOK\n");
harness_stop();
/* Mixed: line 10 was written, so a branch to it stands. */
TEST_REQUIRE_OK(run_program("10 PRINT \"A\"\n"
"PRINT \"B\"\n"
"20 IF A# = 0 THEN A# = 1 : GOTO 10\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "A\nB\nA\nB\n");
harness_stop();
}
/** @brief A branch to an empty line is left alone, as RENUMBER leaves it alone. */
static void test_numeric_branch_to_an_empty_line_is_allowed(void)
{
TEST_REQUIRE_OK(run_program("PRINT \"A\"\n"
"GOTO 500\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "A\n");
harness_stop();
}
/** @brief A label target never reaches the check, so it cannot be refused by it. */
static void test_label_targets_are_never_refused(void)
{
TEST_REQUIRE_OK(run_program("GOTO DONE\n"
"PRINT \"SKIPPED\"\n"
"LABEL DONE\n"
"PRINT \"OK\"\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK\n");
harness_stop();
}
/** @brief A number inside a string literal is not a branch target. */
static void test_a_number_in_a_string_is_not_a_target(void)
{
TEST_REQUIRE_OK(run_program("PRINT \"A\"\n"
"PRINT \"GOTO 2\"\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "A\nGOTO 2\n");
harness_stop();
}
/**
* @brief RENUMBER makes a numberless program branchable by number again.
*
* It moves line 3 to line 30 *and* rewrites `GOTO 3` to `GOTO 30`, and every line
* it touches comes out numbered -- so the branch the check refused a moment ago
* is now naming a number the program owns. That is the remedy the refusal names.
*/
static void test_renumber_makes_numeric_branches_legal(void)
{
TEST_REQUIRE_OK(harness_start(NULL));
TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME,
"PRINT \"A\"\n"
"GOTO 3\n"
"PRINT \"B\"\n"));
/* Before: line 3 was assigned a number, so branching to it is refused. */
TEST_REQUIRE_STATUS(akbasic_runtime_check_targets(&HARNESS_RUNTIME), AKBASIC_ERR_SYNTAX);
TEST_REQUIRE_OK(akbasic_renumber(&HARNESS_RUNTIME, 10, 10, 0));
TEST_REQUIRE_STR(HARNESS_RUNTIME.source[20].code, "GOTO 30");
TEST_REQUIRE_OK(akbasic_runtime_check_targets(&HARNESS_RUNTIME));
harness_stop();
}
/** @brief RENUMBER numbers everything, which is what asking for it means. */
static void test_renumber_marks_every_line_numbered(void)
{
@@ -246,6 +345,12 @@ int main(void)
test_runstream_assigns_the_same_slots();
test_trailing_blank_line_keeps_the_last_line();
test_prompt_still_runs_direct_mode();
test_numeric_branch_to_an_assigned_line_is_refused();
test_numeric_branch_to_a_written_line_is_allowed();
test_numeric_branch_to_an_empty_line_is_allowed();
test_label_targets_are_never_refused();
test_a_number_in_a_string_is_not_a_target();
test_renumber_makes_numeric_branches_legal();
test_renumber_marks_every_line_numbered();
return akbasic_test_failures;
}