From 455c733e5683de37217c677e1f753cad31213069 Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Wed, 25 Jan 2023 19:43:08 +0200 Subject: [PATCH 1/3] Allow I/O operations to have a level of indirection. This pull-request supersedes #121, which was never merged, as a solution to embedding the interpreter and allowing flexible I/O handling: * Rather than changing our builtin-signatures we introduce a new config/ package. * We pass an instance of the configuration object, containing handles, to the interpreter. * This is then accessed to read/write to STDIN/STDOUT. The test-case for (print), TestPrint, was updated to send output to a faux I/O helper - which is actually a buffer - and shows that the indirection works. And now that we've implemented the I/O helper we've updated (read) to use it appropriately too. --- builtins/builtins.go | 14 +++++++++++--- builtins/builtins_test.go | 37 ++++++++++++++++++++++++++++++++++--- builtins/file_info_unix.go | 1 - env/env.go | 28 +++++++++++++++++++++++++--- eval/eval.go | 14 -------------- eval/eval_test.go | 7 +++++++ eval/specials.go | 6 +++++- fuzz_test.go | 4 ++++ main.go | 4 ++++ 9 files changed, 90 insertions(+), 25 deletions(-) diff --git a/builtins/builtins.go b/builtins/builtins.go index 0260ba2..763ddc2 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -1454,13 +1454,17 @@ func printFn(env *env.Environment, args []primitive.Primitive) primitive.Primiti return primitive.ArityError() } + ioHelper := env.GetIOConfig() + // one arg if len(args) == 1 { // expand str := expandStr(args[0].ToString()) - // show & return - fmt.Println(str) + // Write via our configuration object + ioHelper.STDOUT.Write([]byte(str)) + ioHelper.STDOUT.Write([]byte("\n")) + return primitive.String(str) } @@ -1481,7 +1485,11 @@ func printFn(env *env.Environment, args []primitive.Primitive) primitive.Primiti } out := fmt.Sprintf(frmt, parm...) - fmt.Println(out) + + // Write via our configuration object + ioHelper.STDOUT.Write([]byte(out)) + ioHelper.STDOUT.Write([]byte("\n")) + return primitive.String(out) } diff --git a/builtins/builtins_test.go b/builtins/builtins_test.go index 9821987..1d4b288 100644 --- a/builtins/builtins_test.go +++ b/builtins/builtins_test.go @@ -1,6 +1,7 @@ package builtins import ( + "bytes" "math" "os" "path/filepath" @@ -9,6 +10,7 @@ import ( "testing" "time" + "github.com/skx/yal/config" "github.com/skx/yal/env" "github.com/skx/yal/eval" "github.com/skx/yal/primitive" @@ -21,6 +23,10 @@ var ENV *env.Environment // init ensures our environment pointer is up to date. func init() { ENV = env.New() + + // Environment will have a config + ENV.SetIOConfig(config.DefaultIO()) + } func TestArch(t *testing.T) { @@ -3046,6 +3052,31 @@ func TestPrint(t *testing.T) { t.Fatalf("got string, but wrong one %v", e2) } + // Preserve the current config + orig := ENV.GetIOConfig() + + // Setup a new STDOUT handle, pointing to a buffer + cfg := config.New() + tmp := new(bytes.Buffer) + cfg.STDOUT = tmp + ENV.SetIOConfig(cfg) + + // Print something + out = printFn(ENV, []primitive.Primitive{ + primitive.String("Hello %s!"), + primitive.List{ + primitive.String("world"), + primitive.Number(42), + }, + }) + + // Restore our handles + ENV.SetIOConfig(orig) + + // Our buffer should be good + if tmp.String() != "Hello (world 42)!\n" { + t.Fatalf("(print 'hello (list)!') failed, got '%s'", tmp.String()) + } } // TestRandom tests (random) @@ -3524,7 +3555,7 @@ func TestStringLt(t *testing.T) { } } -func TestTrig(t *testing.T ) { +func TestTrig(t *testing.T) { funs := []primitive.GolangPrimitiveFn{ acosFn, @@ -3538,9 +3569,9 @@ func TestTrig(t *testing.T ) { tanhFn, } - for _, fn := range(funs) { + for _, fn := range funs { - out := fn(nil, []primitive.Primitive{} ) + out := fn(nil, []primitive.Primitive{}) // Will lead to an error e, ok := out.(primitive.Error) diff --git a/builtins/file_info_unix.go b/builtins/file_info_unix.go index 9adf8e1..d718f8d 100644 --- a/builtins/file_info_unix.go +++ b/builtins/file_info_unix.go @@ -18,7 +18,6 @@ func getGID(info os.FileInfo) (int, error) { return int(stat.Gid), nil } - // getUID returns the owner of the file, from the extended information // available after a stat - that is not portable to Windows though. // diff --git a/env/env.go b/env/env.go index f8e0124..8da8866 100644 --- a/env/env.go +++ b/env/env.go @@ -9,6 +9,10 @@ // or call-frames, you can create a nested environment via NewEnvironment. package env +import ( + "github.com/skx/yal/config" +) + // Environment holds our state type Environment struct { @@ -17,6 +21,10 @@ type Environment struct { // values holds the actual values values map[string]any + + // ioconfig holds the interface to the outside world, + // which is used for I/O + ioconfig *config.Config } // Get retrieves a value from the environment. @@ -59,7 +67,8 @@ func (env *Environment) Items() map[string]any { // New creates a new environment, with no parent. func New() *Environment { return &Environment{ - values: map[string]any{}, + values: map[string]any{}, + ioconfig: config.New(), } } @@ -67,8 +76,9 @@ func New() *Environment { // parent environment for values in a higher level. func NewEnvironment(parent *Environment) *Environment { return &Environment{ - parent: parent, - values: map[string]any{}, + parent: parent, + values: map[string]any{}, + ioconfig: parent.ioconfig, } } @@ -87,3 +97,15 @@ func (env *Environment) SetOuter(key string, value any) { env.parent.SetOuter(key, value) } } + +// SetIOConfig updates the configuration object which is stored +// in our environment +func (env *Environment) SetIOConfig(cfg *config.Config) { + env.ioconfig = cfg +} + +// GetIOConfig returns the configuration object which is stored in +// our environment. +func (env *Environment) GetIOConfig() *config.Config { + return env.ioconfig +} diff --git a/eval/eval.go b/eval/eval.go index 6eafb55..8a55564 100644 --- a/eval/eval.go +++ b/eval/eval.go @@ -6,12 +6,10 @@ package eval import ( - "bufio" "context" "errors" "flag" "fmt" - "os" "regexp" "strconv" "strings" @@ -59,14 +57,6 @@ type Eval struct { // The key is the name of the fake method, the value the name of // the field to get/set accessors map[string]string - - // STDIN is an input-reader used for the (read) function, when - // called with no arguments. - STDIN *bufio.Reader - - // STDOUT is the writer which we should use for "(print)", but we - // currently don't. - STDOUT *bufio.Writer } // New constructs a new lisp interpreter. @@ -93,10 +83,6 @@ func New(src string) *Eval { accessors: make(map[string]string), } - // Setup default input/output streams - e.STDIN = bufio.NewReader(os.Stdin) - e.STDOUT = bufio.NewWriter(os.Stdout) - // Setup the default symbol-table (interned) entries. // true diff --git a/eval/eval_test.go b/eval/eval_test.go index 0e9b194..1057fe4 100644 --- a/eval/eval_test.go +++ b/eval/eval_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/skx/yal/builtins" + "github.com/skx/yal/config" "github.com/skx/yal/env" "github.com/skx/yal/primitive" "github.com/skx/yal/stdlib" @@ -26,6 +27,9 @@ func TestAliased(t *testing.T) { // With a new environment env := env.New() + // Environment will have a config + env.SetIOConfig(config.DefaultIO()) + // Populate the default primitives builtins.PopulateEnvironment(env) @@ -416,6 +420,9 @@ a // With a new environment env := env.New() + // Environment will have a config + env.SetIOConfig(config.DefaultIO()) + // Populate the default primitives builtins.PopulateEnvironment(env) diff --git a/eval/specials.go b/eval/specials.go index e5866b0..5ca2aa3 100644 --- a/eval/specials.go +++ b/eval/specials.go @@ -3,6 +3,7 @@ package eval import ( + "bufio" "fmt" "strings" @@ -286,7 +287,10 @@ func (ev *Eval) evalSpecialForm(name string, args []primitive.Primitive, e *env. // zero arguments: read from STDIN if len(args) == 0 { - input, err := ev.STDIN.ReadString('\n') + + ioHelper := e.GetIOConfig() + r := bufio.NewReader(ioHelper.STDIN) + input, err := r.ReadString('\n') if err != nil { return primitive.Error( fmt.Sprintf("failed to read from STDIN %s", err)), true diff --git a/fuzz_test.go b/fuzz_test.go index d106725..34ddfb4 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/skx/yal/builtins" + "github.com/skx/yal/config" "github.com/skx/yal/env" "github.com/skx/yal/eval" "github.com/skx/yal/primitive" @@ -188,6 +189,9 @@ func FuzzYAL(f *testing.F) { // Create a new environment environment := env.New() + // Environment will have a config + environment.SetIOConfig(config.DefaultIO()) + // Populate the default primitives builtins.PopulateEnvironment(environment) diff --git a/main.go b/main.go index 14b0ead..c00dd30 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ import ( "time" "github.com/skx/yal/builtins" + "github.com/skx/yal/config" "github.com/skx/yal/env" "github.com/skx/yal/eval" "github.com/skx/yal/primitive" @@ -43,6 +44,9 @@ func create() { // Create a new environment ENV = env.New() + // Setup the I/O + ENV.SetIOConfig(config.DefaultIO()) + // Populate the default primitives builtins.PopulateEnvironment(ENV) From 58a38b8a8064b0da60f4baf0249af55650113cef Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Wed, 25 Jan 2023 19:54:29 +0200 Subject: [PATCH 2/3] Resolved some warnings, but not the ones I see on github actions. Weird. --- builtins/builtins.go | 10 ++++++---- builtins/builtins_test.go | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/builtins/builtins.go b/builtins/builtins.go index 763ddc2..e188303 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -1462,8 +1462,9 @@ func printFn(env *env.Environment, args []primitive.Primitive) primitive.Primiti str := expandStr(args[0].ToString()) // Write via our configuration object - ioHelper.STDOUT.Write([]byte(str)) - ioHelper.STDOUT.Write([]byte("\n")) + // Linter complains about ignored return values here.. + _, _ = ioHelper.STDOUT.Write([]byte(str)) + _, _ = ioHelper.STDOUT.Write([]byte("\n")) return primitive.String(str) } @@ -1487,8 +1488,9 @@ func printFn(env *env.Environment, args []primitive.Primitive) primitive.Primiti out := fmt.Sprintf(frmt, parm...) // Write via our configuration object - ioHelper.STDOUT.Write([]byte(out)) - ioHelper.STDOUT.Write([]byte("\n")) + // Linter complains about ignored return values here.. + _, _ = ioHelper.STDOUT.Write([]byte(out)) + _, _ = ioHelper.STDOUT.Write([]byte("\n")) return primitive.String(out) } diff --git a/builtins/builtins_test.go b/builtins/builtins_test.go index 1d4b288..744913e 100644 --- a/builtins/builtins_test.go +++ b/builtins/builtins_test.go @@ -3062,7 +3062,7 @@ func TestPrint(t *testing.T) { ENV.SetIOConfig(cfg) // Print something - out = printFn(ENV, []primitive.Primitive{ + printFn(ENV, []primitive.Primitive{ primitive.String("Hello %s!"), primitive.List{ primitive.String("world"), From 40ca1cafdfc3eaf6a6a972258e3c0c50844468df Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Wed, 25 Jan 2023 20:13:34 +0200 Subject: [PATCH 3/3] Added the package we're now using. --- config/config.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 config/config.go diff --git a/config/config.go b/config/config.go new file mode 100644 index 0000000..af77ca5 --- /dev/null +++ b/config/config.go @@ -0,0 +1,47 @@ +// Package config provides an I/O abstraction for our interpreter, +// allowing it to be embedded and used in places where STDIN and STDOUT +// are not necessarily terminal-based. +// +// All input-reading uses the level of indirection provided here, and +// similarly output goes via the writer we hold here. +// +// This abstraction allows a host program to setup a different pair of +// streams prior to initializing the interpreter. +package config + +import ( + "io" + "os" +) + +// Config is a holder for configuration which is used for interfacing +// the interpreter with the outside world. +type Config struct { + + // STDIN is an input-reader used for the (read) function, when + // called with no arguments. + STDIN io.Reader + + // STDOUT is the writer which is used for "(print)". + STDOUT io.Writer +} + +// New returns a new configuration object +func New() *Config { + + e := &Config{} + return e +} + +// DefaultIO returns a configuration which uses the default +// input and output streams - i.e. STDIN and STDOUT work as +// expected +func DefaultIO() *Config { + e := New() + + // Setup default input/output streams + e.STDIN = os.Stdin + e.STDOUT = os.Stdout + + return e +}