From 8cc78f5f9fc74c543df79e090577de0c074fcf6c Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Sat, 16 Dec 2023 05:31:21 +0200 Subject: [PATCH 1/2] 122 - Fix swallowline It was reported that the following program fails, due to issues with swallowing newlines: ``` REM This is a comment PRINT "OK" ``` This comes about because the `swallowLine` implementation we have consumes the newline it uses to stop its search, however the interpreter then goes on to increment the line: func (e *Interpreter) RunOnce() error { .. case token.REM: err = e.swallowLine() case .. .. // // Ready for the next instruction // e.offset++ // // Error? // if err != nil { return err } return nil As an easy fix for the moment I've updated the swallowLine implementation to actually return with the current token being the newline, not the one after that: * The interpreter will already silently NOP an inline newline. * This shouldn't break the IF/THEN/ELSE code. A similar solution would have been to add a break after the token.REM handler, but special-casing that felt wrong. This closes #122. --- eval/eval.go | 35 ++++++++++++++++++----------------- eval/eval_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/eval/eval.go b/eval/eval.go index 2234240..d27fc76 100644 --- a/eval/eval.go +++ b/eval/eval.go @@ -10,7 +10,6 @@ // as REM, DATA, READ, etc. Things that could be pushed outside the core, // such as the maths-primitives (SIN, COS, TAN, etc) have been moved into // their own package to keep this as simple and readable as possible. -// package eval import ( @@ -597,7 +596,8 @@ func (e *Interpreter) factor() object.Object { } // terminal - handles parsing of the form -// ARG1 OP ARG2 +// +// ARG1 OP ARG2 // // See also expr() which is similar. func (e *Interpreter) term() object.Object { @@ -773,7 +773,9 @@ func (e *Interpreter) term() object.Object { } // expression - handles parsing of the form -// ARG1 OP ARG2 +// +// ARG1 OP ARG2 +// // See also term() which is similar. func (e *Interpreter) expr(allowBinOp bool) object.Object { @@ -1811,8 +1813,9 @@ func (e *Interpreter) runGOTO() error { // runINPUT handles input of numbers from the user. // // NOTE: -// INPUT "Foo", a -> Reads an integer -// INPUT "Foo", a$ -> Reads a string +// +// INPUT "Foo", a -> Reads an integer +// INPUT "Foo", a$ -> Reads a string func (e *Interpreter) runINPUT() error { // Skip the INPUT-instruction @@ -1909,10 +1912,9 @@ func (e *Interpreter) runINPUT() error { // // Here we _only_ allow: // -// IF $EXPR THEN $STATEMENT ELSE $STATEMENT NEWLINE +// IF $EXPR THEN $STATEMENT ELSE $STATEMENT NEWLINE // // $STATEMENT will only be a single expression -// func (e *Interpreter) runIF() error { // Bump past the IF token @@ -2291,19 +2293,21 @@ func (e *Interpreter) runNEXT() error { // // This is used by: // -// REM -// DATA -// DEF FN -// +// REM +// DATA +// DEF FN func (e *Interpreter) swallowLine() error { - run := true + // Look forwards + for e.offset < len(e.program) { - for e.offset < len(e.program) && run { + // If the token is a newline, or EOF we're done tok := e.program[e.offset] if tok.Type == token.NEWLINE || tok.Type == token.EOF { - run = false + return nil } + + // Otherwise keep going. e.offset++ } @@ -2786,7 +2790,6 @@ func (e *Interpreter) GetTrace() bool { // SetVariable sets the contents of a variable in the interpreter environment. // // Useful for testing/embedding. -// func (e *Interpreter) SetVariable(id string, val object.Object) { e.vars.Set(id, val) } @@ -2844,7 +2847,6 @@ func (e *Interpreter) SetArrayVariable(id string, index []int, val object.Object // GetVariable returns the contents of the given variable. // // Useful for testing/embedding. -// func (e *Interpreter) GetVariable(id string) object.Object { val := e.vars.Get(id) @@ -2888,7 +2890,6 @@ func (e *Interpreter) GetArrayVariable(id string, index []int) object.Object { // be called from the users' BASIC program. // // Useful for embedding. -// func (e *Interpreter) RegisterBuiltin(name string, nArgs int, ft builtin.Signature) { // diff --git a/eval/eval_test.go b/eval/eval_test.go index 3c7c2d3..76aded2 100644 --- a/eval/eval_test.go +++ b/eval/eval_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/skx/gobasic/object" + "github.com/skx/gobasic/token" "github.com/skx/gobasic/tokenizer" ) @@ -1730,3 +1731,38 @@ func TestZero(t *testing.T) { } } + +// TestSwallowLine tests we don't eat too many tokens in the processing +// of newlines. +func TestSwallowLine(t *testing.T) { + + input := `10 REM "This is a test" So is this +20 PRINT "OK" +` + + tokener := tokenizer.New(input) + e, err := New(tokener) + if err != nil { + t.Errorf("Error parsing %s - %s", input, err.Error()) + } + + // We start at offset 0 + if e.offset != 0 { + t.Fatalf("we didn't start at the beginning") + } + + err = e.swallowLine() + if err != nil { + t.Fatalf("error eating line") + } + + // offset should now be bigger + if e.offset != 6 { + t.Fatalf("our offset was %d not %d", e.offset, 6) + } + + // And we should have a newline as the next token + if e.program[e.offset].Type != token.NEWLINE { + t.Fatalf("did not get a line number got %v", e.program[e.offset]) + } +} From d698eb72e6ec6cab3b4abe318d81b503e3c94372 Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Sat, 16 Dec 2023 05:40:23 +0200 Subject: [PATCH 2/2] Updated the helper scripts to avoid CI-failures --- .github/build | 3 +++ .github/run-tests.sh | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/.github/build b/.github/build index cf8bd4e..d9b2710 100755 --- a/.github/build +++ b/.github/build @@ -3,6 +3,9 @@ # The basename of our binary BASE="gobasic" +# I don't even .. +go env -w GOFLAGS="-buildvcs=false" + # Get the dependencies go mod init diff --git a/.github/run-tests.sh b/.github/run-tests.sh index b22ecec..5c2761e 100755 --- a/.github/run-tests.sh +++ b/.github/run-tests.sh @@ -1,5 +1,10 @@ #!/bin/bash + +# I don't even .. +go env -w GOFLAGS="-buildvcs=false" + + # Install the tools we use to test our code-quality. # # Here we setup the tools to install only if the "CI" environmental variable