From ca88c809621c6d02266b8f35482b821bec81c5c3 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 14:13:43 -0500 Subject: [PATCH 01/16] XXX: newscope iteration with sam. passes! wow --- lang/ast/structs.go | 267 +++++++++++++++++++++-------------------- lang/interfaces/ast.go | 2 +- 2 files changed, 136 insertions(+), 133 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 5c737cc270..37d0f16d81 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3561,143 +3561,61 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // collect all the bind statements in the first pass // this allows them to appear out of order in this scope binds := make(map[string]struct{}) // bind existence in this scope - for _, x := range obj.Body { - bind, ok := x.(*StmtBind) - if !ok { - continue - } - // check for duplicates *in this scope* - if _, exists := binds[bind.Ident]; exists { - return fmt.Errorf("var `%s` already exists in this scope", bind.Ident) - } - - binds[bind.Ident] = struct{}{} // mark as found in scope - // add to scope, (overwriting, aka shadowing is ok) - newScope.Variables[bind.Ident] = &ExprTopLevel{ - Definition: &ExprSingleton{ - Definition: bind.Value, - - mutex: &sync.Mutex{}, // TODO: call Init instead - }, - CapturedScope: newScope, - } - if obj.data.Debug { // TODO: is this message ever useful? - obj.data.Logf("prog: set scope: bind collect: (%+v): %+v (%T) is %p", bind.Ident, bind.Value, bind.Value, bind.Value) - } - } - // now collect all the functions, and group by name (if polyfunc is ok) functions := make(map[string][]*StmtFunc) - for _, x := range obj.Body { - fn, ok := x.(*StmtFunc) - if !ok { - continue - } - - _, exists := functions[fn.Name] - if !exists { - functions[fn.Name] = []*StmtFunc{} // initialize - } - - // check for duplicates *in this scope* - if exists && !AllowUserDefinedPolyFunc { - return fmt.Errorf("func `%s` already exists in this scope", fn.Name) - } - - // collect functions (if multiple, this is a polyfunc) - functions[fn.Name] = append(functions[fn.Name], fn) - } - - for name, fnList := range functions { - if obj.data.Debug { // TODO: is this message ever useful? - obj.data.Logf("prog: set scope: collect: (%+v -> %d): %+v (%T)", name, len(fnList), fnList[0].Func, fnList[0].Func) - } - // add to scope, (overwriting, aka shadowing is ok) - if len(fnList) == 1 { - fn := fnList[0].Func // local reference to avoid changing it in the loop... - // add to scope, (overwriting, aka shadowing is ok) - newScope.Functions[name] = &ExprPoly{ // XXX: is this ExprPoly approach optimal? - Definition: &ExprTopLevel{ - Definition: fn, // store the *ExprFunc - CapturedScope: newScope, - }, - } - continue - } - - // build polyfunc's - // XXX: not implemented - return fmt.Errorf("user-defined polyfuncs of length %d are not supported", len(fnList)) - } // now collect any classes // TODO: if we ever allow poly classes, then group in lists by name classes := make(map[string]struct{}) - for _, x := range obj.Body { - class, ok := x.(*StmtClass) - if !ok { - continue - } - // check for duplicates *in this scope* - if _, exists := classes[class.Name]; exists { - return fmt.Errorf("class `%s` already exists in this scope", class.Name) - } - - classes[class.Name] = struct{}{} // mark as found in scope - // add to scope, (overwriting, aka shadowing is ok) - newScope.Classes[class.Name] = class - } - - obj.scope = newScope // save a reference in case we're read by an import - - // This is the legacy variant of this function that doesn't allow - // out-of-order code. It also returns obscure error messages for some - // cases, such as double-recursion. It's left here for reference. - if legacyProgSetScope { - // first set the scope on the classes, since it gets used in include... - for _, stmt := range obj.Body { - //if _, ok := stmt.(*StmtClass); !ok { - // continue - //} - _, ok1 := stmt.(*StmtClass) - _, ok2 := stmt.(*StmtFunc) // TODO: is this correct? - _, ok3 := stmt.(*StmtBind) // TODO: is this correct? - if !ok1 && !ok2 && !ok3 { // if all are false, we skip - continue - } - if obj.data.Debug { - obj.data.Logf("prog: set scope: pass 1: %+v", stmt) - } - if err := stmt.SetScope(newScope); err != nil { - return err - } - } - - // now set the child scopes... - for _, stmt := range obj.Body { - // NOTE: We used to skip over *StmtClass here for recursion... - // Skip over *StmtClass here, since we already did it above... - if _, ok := stmt.(*StmtClass); ok { - continue - } - if _, ok := stmt.(*StmtFunc); ok { // TODO: is this correct? - continue - } - if _, ok := stmt.(*StmtBind); ok { // TODO: is this correct? - continue - } + // // This is the legacy variant of this function that doesn't allow + // // out-of-order code. It also returns obscure error messages for some + // // cases, such as double-recursion. It's left here for reference. + // if legacyProgSetScope { + // // first set the scope on the classes, since it gets used in include... + // for _, stmt := range obj.Body { + // //if _, ok := stmt.(*StmtClass); !ok { + // // continue + // //} + // _, ok1 := stmt.(*StmtClass) + // _, ok2 := stmt.(*StmtFunc) // TODO: is this correct? + // _, ok3 := stmt.(*StmtBind) // TODO: is this correct? + // if !ok1 && !ok2 && !ok3 { // if all are false, we skip + // continue + // } + + // if obj.data.Debug { + // obj.data.Logf("prog: set scope: pass 1: %+v", stmt) + // } + // if err := stmt.SetScope(newScope); err != nil { + // return err + // } + // } - if obj.data.Debug { - obj.data.Logf("prog: set scope: pass 2: %+v", stmt) - } - if err := stmt.SetScope(newScope); err != nil { - return err - } - } + // // now set the child scopes... + // for _, stmt := range obj.Body { + // // NOTE: We used to skip over *StmtClass here for recursion... + // // Skip over *StmtClass here, since we already did it above... + // if _, ok := stmt.(*StmtClass); ok { + // continue + // } + // if _, ok := stmt.(*StmtFunc); ok { // TODO: is this correct? + // continue + // } + // if _, ok := stmt.(*StmtBind); ok { // TODO: is this correct? + // continue + // } + + // if obj.data.Debug { + // obj.data.Logf("prog: set scope: pass 2: %+v", stmt) + // } + // if err := stmt.SetScope(newScope); err != nil { + // return err + // } + // } - return nil - } + // return nil + // } // TODO: this could be called once at the top-level, and then cached... // TODO: it currently gets called inside child programs, which is slow! @@ -3775,6 +3693,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // un-consumed statements to be skipped. As a result, this simplifies // the graph significantly in cases of unused code, because they're not // given a chance to SetScope even though they're in the StmtProg list. + loopScope := newScope.Copy() + funcCount := make(map[string]int) for _, x := range nodeOrder { // these are in the correct order for SetScope stmt, ok := x.(interfaces.Stmt) if !ok { @@ -3787,13 +3707,96 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // Skip any unwanted additions that we pulled in. continue } - if obj.data.Debug { - obj.data.Logf("prog: set scope: order: %+v", stmt) - } - if err := stmt.SetScope(newScope); err != nil { + + capturedScope := loopScope.Copy() + if err := stmt.SetScope(capturedScope); err != nil { return err } + + if bind, ok := x.(*StmtBind); ok { + // check for duplicates *in this scope* + if _, exists := binds[bind.Ident]; exists { + return fmt.Errorf("var `%s` already exists in this scope", bind.Ident) + } + + binds[bind.Ident] = struct{}{} // mark as found in scope + // add to scope, (overwriting, aka shadowing is ok) + loopScope.Variables[bind.Ident] = &ExprTopLevel{ + Definition: &ExprSingleton{ + Definition: bind.Value, + + mutex: &sync.Mutex{}, // TODO: call Init instead + }, + CapturedScope: capturedScope, + } + if obj.data.Debug { // TODO: is this message ever useful? + obj.data.Logf("prog: set scope: bind collect: (%+v): %+v (%T) is %p", bind.Ident, bind.Value, bind.Value, bind.Value) + } + + continue // optional + } + + if fn, ok := x.(*StmtFunc); ok { + _, exists := functions[fn.Name] + if !exists { + functions[fn.Name] = []*StmtFunc{} // initialize + } + + // check for duplicates *in this scope* + if exists && !AllowUserDefinedPolyFunc { + return fmt.Errorf("func `%s` already exists in this scope", fn.Name) + } + + count := 1 // XXX: number of overloaded definitions of the same name (get from ordering eventually) + funcCount[fn.Name]++ + + // collect functions (if multiple, this is a polyfunc) + functions[fn.Name] = append(functions[fn.Name], fn) + + if funcCount[fn.Name] < count { + continue // delay SetScope for later... + } + + fnList := functions[fn.Name] // []*StmtFunc + name := fn.Name // XXX PORT + + if obj.data.Debug { // TODO: is this message ever useful? + obj.data.Logf("prog: set scope: collect: (%+v -> %d): %+v (%T)", name, len(fnList), fnList[0].Func, fnList[0].Func) + } + + // add to scope, (overwriting, aka shadowing is ok) + if len(fnList) == 1 { + fn := fnList[0].Func // local reference to avoid changing it in the loop... + // add to scope, (overwriting, aka shadowing is ok) + loopScope.Functions[name] = &ExprPoly{ // XXX: is this ExprPoly approach optimal? + Definition: &ExprTopLevel{ + Definition: fn, // store the *ExprFunc + CapturedScope: capturedScope, + }, + } + continue + } + + // build polyfunc's + // XXX: not implemented + return fmt.Errorf("user-defined polyfuncs of length %d are not supported", len(fnList)) + } + + if class, ok := x.(*StmtClass); ok { + // check for duplicates *in this scope* + if _, exists := classes[class.Name]; exists { + return fmt.Errorf("class `%s` already exists in this scope", class.Name) + } + + classes[class.Name] = struct{}{} // mark as found in scope + + // add to scope, (overwriting, aka shadowing is ok) + loopScope.Classes[class.Name] = class + } } + + obj.scope = loopScope // save a reference in case we're read by an import + if obj.data.Debug { obj.data.Logf("prog: set scope: finished") } diff --git a/lang/interfaces/ast.go b/lang/interfaces/ast.go index ebb617f5ac..0dd0ec12e4 100644 --- a/lang/interfaces/ast.go +++ b/lang/interfaces/ast.go @@ -235,7 +235,7 @@ type Data struct { // from the variables, which could actually contain lambda functions. type Scope struct { Variables map[string]Expr - Functions map[string]Expr // the Expr will usually be an *ExprFunc + Functions map[string]Expr // the Expr will usually be an *ExprFunc (actually it's usually (or always) an *ExprSingleton, which wraps an *ExprFunc now) Classes map[string]Stmt Chain []Node // chain of previously seen node's From 9b83ef12e815f93c07c5915cc25ad845127567f1 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 14:17:17 -0500 Subject: [PATCH 02/16] XXX: wip classes stuff --- lang/ast/structs.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 37d0f16d81..cbbab7732d 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -65,6 +65,12 @@ const ( // MetaField is the prefix used to specify a meta parameter for the res. MetaField = "meta" + // AllowBareIncluding specifies that a simple include without an `as` + // suffix, will be pulled in under the name of the included class. We + // want this on if it turns out to be common to pull in values from + // classes. + AllowBareIncluding = false + // AllowUserDefinedPolyFunc specifies if we allow user-defined // polymorphic functions or not. At the moment this is not implemented. // XXX: not implemented @@ -4299,8 +4305,9 @@ type StmtInclude struct { class *StmtClass // copy of class that we're using orig *StmtInclude // original pointer to this - Name string - Args []interfaces.Expr + Name string + Args []interfaces.Expr + Alias string } // String returns a short representation of this statement. @@ -4371,9 +4378,10 @@ func (obj *StmtInclude) Interpolate() (interfaces.Stmt, error) { } return &StmtInclude{ //class: obj.class, // TODO: is this necessary? - orig: orig, - Name: obj.Name, - Args: args, + orig: orig, + Name: obj.Name, + Args: args, + Alias: obj.Alias, }, nil } @@ -4406,9 +4414,10 @@ func (obj *StmtInclude) Copy() (interfaces.Stmt, error) { } return &StmtInclude{ //class: obj.class, // TODO: is this necessary? - orig: orig, - Name: obj.Name, - Args: args, + orig: orig, + Name: obj.Name, + Args: args, + Alias: obj.Alias, }, nil } @@ -4425,6 +4434,7 @@ func (obj *StmtInclude) Ordering(produces map[string]interfaces.Node) (*pgraph.G if obj.Name == "" { return nil, nil, fmt.Errorf("missing class name") } + // TODO: do we want obj.Alias added in here? uid := classOrderingPrefix + obj.Name // ordering id cons := make(map[interfaces.Node]string) From b654a3ba2f2af7d2e1324175668d604f8a7e50fa Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 17:04:07 -0500 Subject: [PATCH 03/16] lang: interfaces, parser, ast: Add a bare import constant And add the permissivity of it behind a compile-time flag. --- lang/ast/structs.go | 21 ++++++++++++++++++--- lang/interfaces/const.go | 6 ++++++ lang/parser/lexer.nex | 6 ++++++ lang/parser/lexparse_test.go | 4 ++-- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index cbbab7732d..eb4d579a96 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -71,6 +71,12 @@ const ( // classes. AllowBareIncluding = false + // AllowBareImports specifies that you're allowed to use an import which + // flattens the imported scope on top of the current scope. This means + // imports of the form: `import foo as *`. These are being provisionally + // enabled, despite being less explicit and harder to parse. + AllowBareImports = true + // AllowUserDefinedPolyFunc specifies if we allow user-defined // polymorphic functions or not. At the moment this is not implemented. // XXX: not implemented @@ -3524,7 +3530,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // TODO: do this in a deterministic (sorted) order for name, x := range importedScope.Variables { newName := alias + interfaces.ModuleSep + name - if alias == "*" { + if alias == interfaces.BareSymbol { + if !AllowBareImports { + return fmt.Errorf("bare imports disabled at compile time for import of `%s`", imp.Name) + } newName = name } if previous, exists := newVariables[newName]; exists { @@ -3536,7 +3545,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } for name, x := range importedScope.Functions { newName := alias + interfaces.ModuleSep + name - if alias == "*" { + if alias == interfaces.BareSymbol { + if !AllowBareImports { + return fmt.Errorf("bare imports disabled at compile time for import of `%s`", imp.Name) + } newName = name } if previous, exists := newFunctions[newName]; exists { @@ -3548,7 +3560,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } for name, x := range importedScope.Classes { newName := alias + interfaces.ModuleSep + name - if alias == "*" { + if alias == interfaces.BareSymbol { + if !AllowBareImports { + return fmt.Errorf("bare imports disabled at compile time for import of `%s`", imp.Name) + } newName = name } if previous, exists := newClasses[newName]; exists { diff --git a/lang/interfaces/const.go b/lang/interfaces/const.go index e92d26753a..45761c498f 100644 --- a/lang/interfaces/const.go +++ b/lang/interfaces/const.go @@ -28,6 +28,12 @@ const ( // also used with `ModuleSep` for scoped variables like `$foo.bar.baz`. VarPrefix = "$" + // BareSymbol is the character used primarily for imports to specify + // that we want to import the entire contents and flatten them into our + // current scope. It should probably be removed entirely to force + // explicit imports. + BareSymbol = "*" + // PanicResKind is the kind string used for the panic resource. PanicResKind = "_panic" ) diff --git a/lang/parser/lexer.nex b/lang/parser/lexer.nex index ca6e716f45..2318b27046 100644 --- a/lang/parser/lexer.nex +++ b/lang/parser/lexer.nex @@ -85,8 +85,14 @@ return MINUS } /\*/ { + // This is used as the multiplication symbol, but also + // (for now) the bare import feature, eg: `import as *`. yylex.pos(lval) // our pos lval.str = yylex.Text() + // sanity check... these should be the same! + if x, y := lval.str, interfaces.BareSymbol; x != y { + panic(fmt.Sprintf("MULTIPLY does not match BareSymbol (%s != %s)", x, y)) + } return MULTIPLY } /\// { diff --git a/lang/parser/lexparse_test.go b/lang/parser/lexparse_test.go index fde80de438..761a2128f6 100644 --- a/lang/parser/lexparse_test.go +++ b/lang/parser/lexparse_test.go @@ -1670,12 +1670,12 @@ func TestLexParse0(t *testing.T) { exp: exp, }) } - { + if ast.AllowBareImports { exp := &ast.StmtProg{ Body: []interfaces.Stmt{ &ast.StmtImport{ Name: "foo1", - Alias: "*", + Alias: interfaces.BareSymbol, }, }, } From 17413846d081c9bb44bb45e22f8f14c7bfd43667 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 14:34:52 -0500 Subject: [PATCH 04/16] XXX: wip includes stuff --- lang/ast/structs.go | 88 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index eb4d579a96..f02565d05d 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3589,6 +3589,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // TODO: if we ever allow poly classes, then group in lists by name classes := make(map[string]struct{}) + //includes := make(map[string]struct{}) + // // This is the legacy variant of this function that doesn't allow // // out-of-order code. It also returns obscure error messages for some // // cases, such as double-recursion. It's left here for reference. @@ -3813,7 +3815,93 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // add to scope, (overwriting, aka shadowing is ok) loopScope.Classes[class.Name] = class + + continue + } + + // now collect any include contents + if include, ok := x.(*StmtInclude); ok { + // XXX: actually we don't want to check for duplicates, that's allowed... What about include foo as bar twice? + // XXX: or what about `include foo(true) as bar; include foo(false) as bar` ? + // check for duplicates *in this scope* + //if _, exists := includes[include.Name]; exists { + // return fmt.Errorf("include `%s` already exists in this scope", include.Name) + //} + + alias := "" + if AllowBareIncluding { + alias = include.Name // this is what we would call the include + } + if include.Alias != "" { // this is what the user decided as the name + alias = include.Alias // use alias if specified + } + if alias == "" { + continue // there isn't anything to do here + } + + // XXX: deal with alias duplicates and * includes and so on... + //if _, exists := aliases[alias]; exists { + // TODO: track separately to give a better error message here + // return fmt.Errorf("import/include alias `%s` already exists in this scope", alias) + //} + + // XXX: is this failing because this code should be in StmtInclude:SetScope ? + // XXX: fundamentally this happens because we didn't do SetScope on StmtInclude yet? + if include.class == nil { + // programming error + return fmt.Errorf("programming error: class `%s` not found", include.Name) + } + //scope := include.class.scope // this includes the $x in the tricky case, but we'll allow that for now + //includedScope := scope.Copy() // XXX ??? + includedScope := include.class.scope + + // read from stored scope which was previously saved in SetScope + // add to scope, (overwriting, aka shadowing is ok) + // rename scope values, adding the alias prefix + // check that we don't overwrite a new value from another include + // TODO: do this in a deterministic (sorted) order + for name, x := range includedScope.Variables { + newName := alias + interfaces.ModuleSep + name + if alias == "*" { // XXX: not supported by parser atm! + newName = name + } + if previous, exists := newVariables[newName]; exists { + // don't overwrite in same scope + return fmt.Errorf("can't squash variable `%s` from `%s` by include of `%s`", newName, previous, include.Name) + } + newVariables[newName] = include.Name + loopScope.Variables[newName] = x // merge + } + for name, x := range includedScope.Functions { + newName := alias + interfaces.ModuleSep + name + if alias == "*" { // XXX: not supported by parser atm! + newName = name + } + if previous, exists := newFunctions[newName]; exists { + // don't overwrite in same scope + return fmt.Errorf("can't squash function `%s` from `%s` by include of `%s`", newName, previous, include.Name) + } + newFunctions[newName] = include.Name + loopScope.Functions[newName] = x + } + for name, x := range includedScope.Classes { + newName := alias + interfaces.ModuleSep + name + if alias == "*" { // XXX: not supported by parser atm! + newName = name + } + if previous, exists := newClasses[newName]; exists { + // don't overwrite in same scope + return fmt.Errorf("can't squash class `%s` from `%s` by include of `%s`", newName, previous, include.Name) + } + newClasses[newName] = include.Name + loopScope.Classes[newName] = x + } + + // everything has been merged, move on to next include... + //includes[include.Name] = struct{}{} // mark as found in scope + //aliases[alias] = struct{}{} } + } obj.scope = loopScope // save a reference in case we're read by an import From 2dd0c1662e00357410c33fd685968cbac54729d5 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 15:10:13 -0500 Subject: [PATCH 05/16] XXX: class parser changes --- lang/parser/parser.y | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lang/parser/parser.y b/lang/parser/parser.y index 91215a57fb..3501032d3e 100644 --- a/lang/parser/parser.y +++ b/lang/parser/parser.y @@ -295,6 +295,27 @@ stmt: Args: $4.exprs, } } + // `include name as foo` + // TODO: should we support: `include name as *` +| INCLUDE_IDENTIFIER dotted_identifier AS_IDENTIFIER IDENTIFIER + { + posLast(yylex, yyDollar) // our pos + $$.stmt = &ast.StmtInclude{ + Name: $2.str, + Alias: $4.str, + } + } + // `include name(...) as foo` + // TODO: should we support: `include name(...) as *` +| INCLUDE_IDENTIFIER dotted_identifier OPEN_PAREN call_args CLOSE_PAREN AS_IDENTIFIER IDENTIFIER + { + posLast(yylex, yyDollar) // our pos + $$.stmt = &ast.StmtInclude{ + Name: $2.str, + Args: $4.exprs, + Alias: $7.str, + } + } // `import "name"` | IMPORT_IDENTIFIER STRING { From cf7c2dcd4cb68ee95e91a044976dd705283322d0 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 15:11:12 -0500 Subject: [PATCH 06/16] XXX: add new tests for class include as stuff --- .../class-include-as-class0.txtar | 21 +++++++++ .../class-include-as-class1.txtar | 39 ++++++++++++++++ .../class-include-as-class2.txtar | 17 +++++++ .../class-include-as-func-scope0.txtar | 14 ++++++ .../class-include-as-func-scope1.txtar | 14 ++++++ .../TestAstFunc3/class-include-as-func0.txtar | 13 ++++++ .../TestAstFunc3/class-include-as-func1.txtar | 27 ++++++++++++ .../TestAstFunc3/class-include-as-func2.txtar | 44 +++++++++++++++++++ .../class-include-as-lambda-scope0.txtar | 14 ++++++ .../class-include-as-lambda-scope1.txtar | 14 ++++++ .../class-include-as-lambda0.txtar | 13 ++++++ .../class-include-as-lambda1.txtar | 27 ++++++++++++ .../class-include-as-lambda2.txtar | 44 +++++++++++++++++++ .../TestAstFunc3/class-include-as-vars0.txtar | 11 +++++ .../TestAstFunc3/class-include-as-vars1.txtar | 25 +++++++++++ .../TestAstFunc3/class-include-as-vars2.txtar | 42 ++++++++++++++++++ 16 files changed, 379 insertions(+) create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar new file mode 100644 index 0000000000..ae50a9dac3 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar @@ -0,0 +1,21 @@ +-- main.mcl -- +class c1 { + test "t1" {} + $x = "hello" + class c0 { + test "t2" {} + $y = "goodbye" + } +} +include c1 as i1 +include i1.c0 as i0 + +test $i0.x {} +test $i1.y {} +panic($i0.x != "hello") +panic($i1.y != "goodbye") +-- OUTPUT -- +Vertex: test[goodbye] +Vertex: test[hello] +Vertex: test[t1] +Vertex: test[t2] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar new file mode 100644 index 0000000000..94b164e3f1 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar @@ -0,0 +1,39 @@ +-- main.mcl -- +class c1($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + class c0 { + test "t4" {} + if $b { + test "t5" {} + } else { + test "t6" {} + } + $x = if $b { + "hello" + } else { + "goodbye" + } + } +} +include c1(true) as i1 +include i1.c0 as i01 + +include c1(false) as i2 +include i2.c0 as i02 + +test $i01.x {} +test $i02.x {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[goodbye] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] +Vertex: test[t4] +Vertex: test[t5] +Vertex: test[t6] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar new file mode 100644 index 0000000000..edff7e84ff --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar @@ -0,0 +1,17 @@ +-- main.mcl -- +class c1($b) { + if $b { # Scope doesn't leak up and out of `if` statement! + class inner() { + test "t1" {} + } + } else { + class inner() { + test "t2" {} + } + } +} + +include c1 as i1 +include i1.inner +-- OUTPUT -- +# err: XXX diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar new file mode 100644 index 0000000000..75b5a0d049 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar @@ -0,0 +1,14 @@ +-- main.mcl -- +class c1 { + $x = "outside" + test "t1" {} + func f1($x) { + "hello" + $x + } +} +include c1 as i1 + +test i1.f1("world") {} +-- OUTPUT -- +Vertex: test[helloworld] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar new file mode 100644 index 0000000000..16c5e30e56 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar @@ -0,0 +1,14 @@ +-- main.mcl -- +class c1 { + $x = "world" + test "t1" {} + func f1($y) { + "hello" + $x + } +} +include c1 as i1 + +test i1.f1("whatever") {} +-- OUTPUT -- +Vertex: test[helloworld] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar new file mode 100644 index 0000000000..1c102e11db --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar @@ -0,0 +1,13 @@ +-- main.mcl -- +class c1 { + test "t1" {} + func f1() { + "hello" + } +} +include c1 as i1 + +test i1.f1() {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar new file mode 100644 index 0000000000..b7f7c102c7 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar @@ -0,0 +1,27 @@ +-- main.mcl -- +class c1($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + func f1() { + if $b { + "hello" + } else { + "goodbye" + } + } +} +include c1(true) as i1 +include c1(false) as i2 + +test i1.f1() {} +test i2.f1() {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[goodbye] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar new file mode 100644 index 0000000000..dd52bc737a --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar @@ -0,0 +1,44 @@ +-- main.mcl -- +class c0($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + func f0() { + if $b { + "hello" + } else { + "goodbye" + } + } +} +class c1($b) { + test "t4" {} + if $b { + test "t5" {} + } else { + test "t6" {} + } + include c0($b) as i0 + func f1() { i0.f0() } +} +include c1(true) as i1 +include c1(false) as i2 + +test i1.f1() {} +test i2.f1() {} +test i1.i0.f0() {} # I think these might work directly too. Do we want them to? +test i2.i0.f0() {} +-- OUTPUT -- +Vertex: test[goodbye] +Vertex: test[goodbye] +Vertex: test[hello] +Vertex: test[hello] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] +Vertex: test[t4] +Vertex: test[t5] +Vertex: test[t6] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar new file mode 100644 index 0000000000..89df2d7c1a --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar @@ -0,0 +1,14 @@ +-- main.mcl -- +class c1 { + $x = "outside" + test "t1" {} + $f1 = func($x) { + "hello" + $x + } +} +include c1 as i1 + +test $i1.f1("world") {} +-- OUTPUT -- +Vertex: test[helloworld] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar new file mode 100644 index 0000000000..4ab28c4a39 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar @@ -0,0 +1,14 @@ +-- main.mcl -- +class c1 { + $x = "world" + test "t1" {} + $f1 = func($y) { + "hello" + $x + } +} +include c1 as i1 + +test $i1.f1("whatever") {} +-- OUTPUT -- +Vertex: test[helloworld] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar new file mode 100644 index 0000000000..fcc6a981ed --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar @@ -0,0 +1,13 @@ +-- main.mcl -- +class c1 { + test "t1" {} + $f1 = func() { + "hello" + } +} +include c1 as i1 + +test $i1.f1() {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar new file mode 100644 index 0000000000..921efaa147 --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar @@ -0,0 +1,27 @@ +-- main.mcl -- +class c1($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + $f1 = func() { + if $b { + "hello" + } else { + "goodbye" + } + } +} +include c1(true) as i1 +include c1(false) as i2 + +test $i1.f1() {} +test $i2.f1() {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[goodbye] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar new file mode 100644 index 0000000000..f25e9240ec --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar @@ -0,0 +1,44 @@ +-- main.mcl -- +class c0($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + $f0 = func() { + if $b { + "hello" + } else { + "goodbye" + } + } +} +class c1($b) { + test "t4" {} + if $b { + test "t5" {} + } else { + test "t6" {} + } + include c0($b) as i0 + $x = i0.f0 +} +include c1(true) as i1 +include c1(false) as i2 + +test $i1.x() {} +test $i2.x() {} +test $i1.i0.f0() {} # I think these should work directly too. Do we want them to? +test $i2.i0.f0() {} +-- OUTPUT -- +Vertex: test[goodbye] +Vertex: test[goodbye] +Vertex: test[hello] +Vertex: test[hello] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] +Vertex: test[t4] +Vertex: test[t5] +Vertex: test[t6] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar new file mode 100644 index 0000000000..a0a645ee3d --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar @@ -0,0 +1,11 @@ +-- main.mcl -- +class c1 { + test "t1" {} + $x = "hello" +} +include c1 as i1 + +test $i1.x {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[t1] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar new file mode 100644 index 0000000000..fb7034f44d --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar @@ -0,0 +1,25 @@ +-- main.mcl -- +class c1($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + $x = if $b { + "hello" + } else { + "goodbye" + } +} +include c1(true) as i1 +include c1(false) as i2 + +test $i1.x {} +test $i2.x {} +-- OUTPUT -- +Vertex: test[hello] +Vertex: test[goodbye] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar new file mode 100644 index 0000000000..f7d1f027da --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar @@ -0,0 +1,42 @@ +-- main.mcl -- +class c0($b) { + test "t1" {} + if $b { + test "t2" {} + } else { + test "t3" {} + } + $y = if $b { + "hello" + } else { + "goodbye" + } +} +class c1($b) { + test "t4" {} + if $b { + test "t5" {} + } else { + test "t6" {} + } + include c0($b) as i0 + $x = $i0.y +} +include c1(true) as i1 +include c1(false) as i2 + +test $i1.x {} +test $i2.x {} +test $i1.i0.y {} # I think these should work directly too. Do we want them to? +test $i2.i0.y {} +-- OUTPUT -- +Vertex: test[goodbye] +Vertex: test[goodbye] +Vertex: test[hello] +Vertex: test[hello] +Vertex: test[t1] +Vertex: test[t2] +Vertex: test[t3] +Vertex: test[t4] +Vertex: test[t5] +Vertex: test[t6] From 0add5c208bbe24a237691e7d4b48f9acebc90fab Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 15:57:24 -0500 Subject: [PATCH 07/16] XXX making some progress on stuff --- lang/ast/structs.go | 47 ++++++++++++------- .../TestAstFunc3/class-include-as-vars3.txtar | 8 ++++ 2 files changed, 38 insertions(+), 17 deletions(-) create mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar diff --git a/lang/ast/structs.go b/lang/ast/structs.go index f02565d05d..b7f210846b 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3731,6 +3731,24 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { continue } + // This is here before SetScope because we *don't* want to go + // into StmtClass, because we don't want to call SetScope on the + // body of the class because we don't know anything about the + // arguments. (Says Sam.) + if class, ok := x.(*StmtClass); ok { + // check for duplicates *in this scope* + if _, exists := classes[class.Name]; exists { + return fmt.Errorf("class `%s` already exists in this scope", class.Name) + } + + classes[class.Name] = struct{}{} // mark as found in scope + + // add to scope, (overwriting, aka shadowing is ok) + loopScope.Classes[class.Name] = class + + continue + } + capturedScope := loopScope.Copy() if err := stmt.SetScope(capturedScope); err != nil { return err @@ -3805,20 +3823,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { return fmt.Errorf("user-defined polyfuncs of length %d are not supported", len(fnList)) } - if class, ok := x.(*StmtClass); ok { - // check for duplicates *in this scope* - if _, exists := classes[class.Name]; exists { - return fmt.Errorf("class `%s` already exists in this scope", class.Name) - } - - classes[class.Name] = struct{}{} // mark as found in scope - - // add to scope, (overwriting, aka shadowing is ok) - loopScope.Classes[class.Name] = class - - continue - } - // now collect any include contents if include, ok := x.(*StmtInclude); ok { // XXX: actually we don't want to check for duplicates, that's allowed... What about include foo as bar twice? @@ -4366,7 +4370,17 @@ func (obj *StmtClass) SetScope(scope *interfaces.Scope) error { if scope == nil { scope = interfaces.EmptyScope() } - obj.scope = scope // store for later + // obj.scope = scope // XXX NOPE + + if err := obj.Body.SetScope(scope); err != nil { + return err + } + prog, ok := obj.Body.(*StmtProg) + if !ok { + return fmt.Errorf("expected a prog") + } + obj.scope = prog.scope // store for later + return nil } @@ -4661,7 +4675,6 @@ func (obj *StmtInclude) SetScope(scope *interfaces.Scope) error { }, CapturedScope: newScope, } - } // recursion detection @@ -4674,7 +4687,7 @@ func (obj *StmtInclude) SetScope(scope *interfaces.Scope) error { // need to use the original scope of the class as it was set as the // basis for this scope, so that we overwrite it only with the arg // changes. - if err := obj.class.Body.SetScope(newScope); err != nil { + if err := obj.class.SetScope(newScope); err != nil { return err } diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar new file mode 100644 index 0000000000..076e873b7e --- /dev/null +++ b/lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar @@ -0,0 +1,8 @@ +-- main.mcl -- +#$wat = "bad1" +class c1($wat) { + test $wat {} +} +include c1("hello") +-- OUTPUT -- +Vertex: test[hello] From f6ceb97a47ab13859523f9079ec4eba33f1ab1ae Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sat, 6 Jan 2024 16:31:14 -0500 Subject: [PATCH 08/16] XXXTESTS --- lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar | 2 +- lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar | 2 -- lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar | 2 +- lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar | 2 -- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar index ae50a9dac3..af514a53fa 100644 --- a/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar +++ b/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar @@ -11,7 +11,7 @@ include c1 as i1 include i1.c0 as i0 test $i0.x {} -test $i1.y {} +test $i1.y {} # XXX sam says it's wrong should not exist panic($i0.x != "hello") panic($i1.y != "goodbye") -- OUTPUT -- diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar index dd52bc737a..4d146d40ba 100644 --- a/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar +++ b/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar @@ -33,8 +33,6 @@ test i1.i0.f0() {} # I think these might work directly too. Do we want them to? test i2.i0.f0() {} -- OUTPUT -- Vertex: test[goodbye] -Vertex: test[goodbye] -Vertex: test[hello] Vertex: test[hello] Vertex: test[t1] Vertex: test[t2] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar index f25e9240ec..db47e88821 100644 --- a/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar +++ b/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar @@ -22,7 +22,7 @@ class c1($b) { test "t6" {} } include c0($b) as i0 - $x = i0.f0 + $x = $i0.f0 } include c1(true) as i1 include c1(false) as i2 diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar index f7d1f027da..6c5ef002d5 100644 --- a/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar +++ b/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar @@ -31,8 +31,6 @@ test $i1.i0.y {} # I think these should work directly too. Do we want them to? test $i2.i0.y {} -- OUTPUT -- Vertex: test[goodbye] -Vertex: test[goodbye] -Vertex: test[hello] Vertex: test[hello] Vertex: test[t1] Vertex: test[t2] From 0f1d113bf523718a9af7e550d8314888248b4df2 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 15:12:05 -0500 Subject: [PATCH 09/16] XXX more tests and misc cleanup --- lang/ast/structs.go | 196 ++++++++---------- .../class-include-as-duplicate.txtar | 10 + .../class-include-as-import1.txtar | 10 + .../class-include-as-import2.txtar | 10 + .../TestAstFunc2/simple-scope-ordering1.txtar | 7 + .../TestAstFunc2/simple-scope-ordering2.txtar | 7 + 6 files changed, 131 insertions(+), 109 deletions(-) create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar create mode 100644 lang/interpret_test/TestAstFunc2/simple-scope-ordering1.txtar create mode 100644 lang/interpret_test/TestAstFunc2/simple-scope-ordering2.txtar diff --git a/lang/ast/structs.go b/lang/ast/structs.go index b7f210846b..150ec89244 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -65,11 +65,33 @@ const ( // MetaField is the prefix used to specify a meta parameter for the res. MetaField = "meta" - // AllowBareIncluding specifies that a simple include without an `as` - // suffix, will be pulled in under the name of the included class. We - // want this on if it turns out to be common to pull in values from + // AllowBareClassIncluding specifies that a simple include without an + // `as` suffix, will be pulled in under the name of the included class. + // We want this on if it turns out to be common to pull in values from // classes. - AllowBareIncluding = false + // + // If we allow bare including of classes, then we have to also prevent + // duplicate class inclusion for many cases. For example: + // + // class c1($s) { + // test $s {} + // $x = "${s}" + // } + // include c1("hey") + // include c1("there") + // test $x {} + // + // What value should $x have? We want to import two useful `test` + // resources, but with a bare import this makes `$x` ambiguous. We'd + // have to detect this and ensure this is a compile time error to use + // it. Being able to allow compatible, duplicate classes is a key + // important feature of the language, and as a result, enabling this + // would probably be disastrous. The fact that the import statement + // allows bare imports is an ergonomic consideration that is allowed + // because duplicate imports aren't essential. As an aside, the use of + // bare imports isn't recommended because it makes it more difficult to + // know where certain things are coming from. + AllowBareClassIncluding = false // AllowBareImports specifies that you're allowed to use an import which // flattens the imported scope on top of the current scope. This means @@ -113,11 +135,6 @@ const ( // classOrderingPrefix is a magic prefix used for the Ordering graph. classOrderingPrefix = "class:" - // legacyProgSetScope enables an old version of the SetScope function - // in StmtProg. Use it for experimentation if you don't want to use the - // Ordering function for some reason. In general, this should be false! - legacyProgSetScope = false - // ErrNoStoredScope is an error that tells us we can't get a scope here. ErrNoStoredScope = interfaces.Error("scope is not stored in this node") @@ -3579,67 +3596,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { aliases[alias] = struct{}{} } - // collect all the bind statements in the first pass - // this allows them to appear out of order in this scope - binds := make(map[string]struct{}) // bind existence in this scope - // now collect all the functions, and group by name (if polyfunc is ok) - functions := make(map[string][]*StmtFunc) - - // now collect any classes - // TODO: if we ever allow poly classes, then group in lists by name - classes := make(map[string]struct{}) - - //includes := make(map[string]struct{}) - - // // This is the legacy variant of this function that doesn't allow - // // out-of-order code. It also returns obscure error messages for some - // // cases, such as double-recursion. It's left here for reference. - // if legacyProgSetScope { - // // first set the scope on the classes, since it gets used in include... - // for _, stmt := range obj.Body { - // //if _, ok := stmt.(*StmtClass); !ok { - // // continue - // //} - // _, ok1 := stmt.(*StmtClass) - // _, ok2 := stmt.(*StmtFunc) // TODO: is this correct? - // _, ok3 := stmt.(*StmtBind) // TODO: is this correct? - // if !ok1 && !ok2 && !ok3 { // if all are false, we skip - // continue - // } - - // if obj.data.Debug { - // obj.data.Logf("prog: set scope: pass 1: %+v", stmt) - // } - // if err := stmt.SetScope(newScope); err != nil { - // return err - // } - // } - - // // now set the child scopes... - // for _, stmt := range obj.Body { - // // NOTE: We used to skip over *StmtClass here for recursion... - // // Skip over *StmtClass here, since we already did it above... - // if _, ok := stmt.(*StmtClass); ok { - // continue - // } - // if _, ok := stmt.(*StmtFunc); ok { // TODO: is this correct? - // continue - // } - // if _, ok := stmt.(*StmtBind); ok { // TODO: is this correct? - // continue - // } - - // if obj.data.Debug { - // obj.data.Logf("prog: set scope: pass 2: %+v", stmt) - // } - // if err := stmt.SetScope(newScope); err != nil { - // return err - // } - // } - - // return nil - // } - // TODO: this could be called once at the top-level, and then cached... // TODO: it currently gets called inside child programs, which is slow! orderingGraph, _, err := obj.Ordering(nil) // XXX: pass in globals from scope? @@ -3711,6 +3667,17 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { obj.data.Logf("prog: set scope: ordering: %+v", stmts) } + // Track all the bind statements, functions, and classes. This is used + // for duplicate checking. These might appear out-of-order as code, but + // are iterated in the topoligically sorted node order. When we collect + // all the functions, we group by name (if polyfunc is ok) and we also + // do something similar for classes. + // TODO: if we ever allow poly classes, then group in lists by name + binds := make(map[string]struct{}) // bind existence in this scope + functions := make(map[string][]*StmtFunc) + classes := make(map[string]struct{}) + //includes := make(map[string]struct{}) // duplicates are allowed + // Optimization: In addition to importantly skipping the parts of the // graph that don't belong in this StmtProg, this also causes // un-consumed statements to be skipped. As a result, this simplifies @@ -3731,24 +3698,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { continue } - // This is here before SetScope because we *don't* want to go - // into StmtClass, because we don't want to call SetScope on the - // body of the class because we don't know anything about the - // arguments. (Says Sam.) - if class, ok := x.(*StmtClass); ok { - // check for duplicates *in this scope* - if _, exists := classes[class.Name]; exists { - return fmt.Errorf("class `%s` already exists in this scope", class.Name) - } - - classes[class.Name] = struct{}{} // mark as found in scope - - // add to scope, (overwriting, aka shadowing is ok) - loopScope.Classes[class.Name] = class - - continue - } - capturedScope := loopScope.Copy() if err := stmt.SetScope(capturedScope); err != nil { return err @@ -3799,19 +3748,18 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } fnList := functions[fn.Name] // []*StmtFunc - name := fn.Name // XXX PORT if obj.data.Debug { // TODO: is this message ever useful? - obj.data.Logf("prog: set scope: collect: (%+v -> %d): %+v (%T)", name, len(fnList), fnList[0].Func, fnList[0].Func) + obj.data.Logf("prog: set scope: collect: (%+v -> %d): %+v (%T)", fn.Name, len(fnList), fnList[0].Func, fnList[0].Func) } // add to scope, (overwriting, aka shadowing is ok) if len(fnList) == 1 { - fn := fnList[0].Func // local reference to avoid changing it in the loop... + f := fnList[0].Func // local reference to avoid changing it in the loop... // add to scope, (overwriting, aka shadowing is ok) - loopScope.Functions[name] = &ExprPoly{ // XXX: is this ExprPoly approach optimal? + loopScope.Functions[fn.Name] = &ExprPoly{ // XXX: is this ExprPoly approach optimal? Definition: &ExprTopLevel{ - Definition: fn, // store the *ExprFunc + Definition: f, // store the *ExprFunc CapturedScope: capturedScope, }, } @@ -3823,6 +3771,20 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { return fmt.Errorf("user-defined polyfuncs of length %d are not supported", len(fnList)) } + if class, ok := x.(*StmtClass); ok { + // check for duplicates *in this scope* + if _, exists := classes[class.Name]; exists { + return fmt.Errorf("class `%s` already exists in this scope", class.Name) + } + + classes[class.Name] = struct{}{} // mark as found in scope + + // add to scope, (overwriting, aka shadowing is ok) + loopScope.Classes[class.Name] = class + + continue + } + // now collect any include contents if include, ok := x.(*StmtInclude); ok { // XXX: actually we don't want to check for duplicates, that's allowed... What about include foo as bar twice? @@ -3833,7 +3795,7 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { //} alias := "" - if AllowBareIncluding { + if AllowBareClassIncluding { alias = include.Name // this is what we would call the include } if include.Alias != "" { // this is what the user decided as the name @@ -3849,15 +3811,35 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // return fmt.Errorf("import/include alias `%s` already exists in this scope", alias) //} - // XXX: is this failing because this code should be in StmtInclude:SetScope ? - // XXX: fundamentally this happens because we didn't do SetScope on StmtInclude yet? if include.class == nil { // programming error return fmt.Errorf("programming error: class `%s` not found", include.Name) } - //scope := include.class.scope // this includes the $x in the tricky case, but we'll allow that for now - //includedScope := scope.Copy() // XXX ??? - includedScope := include.class.scope + // This includes any variable from the top-level scope + // that is visible (and captured) inside the class, and + // re-exported when included with `as`. This is the + // "tricky case", but it turns out it's better this way. + // Example: + // + // $x = "i am x" # i am now top-level + // class c1() { + // $whatever = fmt.printf("i can see: %s", $x) + // } + // include c1 as i1 + // test $i1.x {} # tricky + // test $i1.whatever {} # easy + // + // We want to allow the tricky case to prevent needing + // to write code like: `$x = $x` inside of class c1 to + // get the same effect. + + //includedScope := include.class.Body.scope // conceptually + prog, ok := include.class.Body.(*StmtProg) + if !ok { + return fmt.Errorf("programming error: prog not found in class Body") + } + // XXX: .Copy() ? + includedScope := prog.scope // read from stored scope which was previously saved in SetScope // add to scope, (overwriting, aka shadowing is ok) @@ -4370,16 +4352,12 @@ func (obj *StmtClass) SetScope(scope *interfaces.Scope) error { if scope == nil { scope = interfaces.EmptyScope() } - // obj.scope = scope // XXX NOPE - if err := obj.Body.SetScope(scope); err != nil { - return err - } - prog, ok := obj.Body.(*StmtProg) - if !ok { - return fmt.Errorf("expected a prog") - } - obj.scope = prog.scope // store for later + // We want to capture what was in scope at the definition site of the + // class so that when we `include` the class, the body of the class is + // expanded with the variables which were in scope at the definition + // site and not the variables which were in scope at the include site. + obj.scope = scope // store for later return nil } @@ -4687,7 +4665,7 @@ func (obj *StmtInclude) SetScope(scope *interfaces.Scope) error { // need to use the original scope of the class as it was set as the // basis for this scope, so that we overwrite it only with the arg // changes. - if err := obj.class.SetScope(newScope); err != nil { + if err := obj.class.Body.SetScope(newScope); err != nil { return err } diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar new file mode 100644 index 0000000000..a74d35b4ed --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar @@ -0,0 +1,10 @@ +-- main.mcl -- +class c1($s) { + $x = "got: ${s}" +} + +include c1("hey") as i1 +include c1("there") as ii +test $i1.x {} +-- OUTPUT -- +# err: can't use the same `as`... diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar new file mode 100644 index 0000000000..a4dc51beff --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar @@ -0,0 +1,10 @@ +-- main.mcl -- +import "fmt" as i1 +class c1($s) { + $x = fmt.printf("got: %s", $s) +} + +include c1("hey") as i1 +test $i1.x {} +-- OUTPUT -- +# err: can't use the same `as`... diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar new file mode 100644 index 0000000000..0be6010c14 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar @@ -0,0 +1,10 @@ +-- main.mcl -- +import "fmt" +class c1($s) { + $x = fmt.printf("got: %s", $s) +} + +include c1("hey") as fmt +test $fmt.x {} +-- OUTPUT -- +# err: can't use the same `as`... diff --git a/lang/interpret_test/TestAstFunc2/simple-scope-ordering1.txtar b/lang/interpret_test/TestAstFunc2/simple-scope-ordering1.txtar new file mode 100644 index 0000000000..683685dd95 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/simple-scope-ordering1.txtar @@ -0,0 +1,7 @@ +-- main.mcl -- +# set scope ordering test +$x = "hey" +$y = $x +test $y {} +-- OUTPUT -- +Vertex: test[hey] diff --git a/lang/interpret_test/TestAstFunc2/simple-scope-ordering2.txtar b/lang/interpret_test/TestAstFunc2/simple-scope-ordering2.txtar new file mode 100644 index 0000000000..a53a0e4cb8 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/simple-scope-ordering2.txtar @@ -0,0 +1,7 @@ +-- main.mcl -- +# set scope ordering test +$y = $x +$x = "hey" +test $y {} +-- OUTPUT -- +Vertex: test[hey] From 18c95229a128822431bc69ce3259e6804459b6a6 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 17:15:44 -0500 Subject: [PATCH 10/16] XXX: wip on ordering and misc --- lang/ast/structs.go | 108 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 94 insertions(+), 14 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 150ec89244..2448186c35 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -93,6 +93,12 @@ const ( // know where certain things are coming from. AllowBareClassIncluding = false + // AllowBareIncludes specifies that you're allowed to use an include + // which flattens the included scope on top of the current scope. This + // means includes of the form: `include foo as *`. These are unlikely to + // get enabled for many reasons. + AllowBareIncludes = false + // AllowBareImports specifies that you're allowed to use an import which // flattens the imported scope on top of the current scope. This means // imports of the form: `import foo as *`. These are being provisionally @@ -135,6 +141,10 @@ const ( // classOrderingPrefix is a magic prefix used for the Ordering graph. classOrderingPrefix = "class:" + // scopedOrderingPrefix is a magic prefix used for the Ordering graph. + // It is shared between imports and include as. + scopedOrderingPrefix = "prefix:" + // ErrNoStoredScope is an error that tells us we can't get a scope here. ErrNoStoredScope = interfaces.Error("scope is not stored in this node") @@ -2978,11 +2988,36 @@ func (obj *StmtProg) Ordering(produces map[string]interfaces.Node) (*pgraph.Grap prod := make(map[string]interfaces.Node) for _, x := range obj.Body { - if stmt, ok := x.(*StmtClass); ok { + if stmt, ok := x.(*StmtImport); ok { if stmt.Name == "" { return nil, nil, fmt.Errorf("missing class name") } - uid := classOrderingPrefix + stmt.Name // ordering id + uid := scopedOrderingPrefix + stmt.Name // ordering id + + if stmt.Alias == interfaces.BareSymbol { + // XXX: I think we need to parse these first... + // XXX: Somehow make sure these appear at the + // top of the topo-sort for the StmtProg... + // XXX: Maybe add edges between StmtProg and me? + continue + } + + if stmt.Alias != "" { + uid = scopedOrderingPrefix + stmt.Alias // ordering id + } + + n, exists := prod[uid] + if exists { + return nil, nil, fmt.Errorf("duplicate assignment to `%s`, have: %s", uid, n) + } + prod[uid] = stmt // store + } + + if stmt, ok := x.(*StmtBind); ok { + if stmt.Ident == "" { + return nil, nil, fmt.Errorf("missing bind name") + } + uid := varOrderingPrefix + stmt.Ident // ordering id n, exists := prod[uid] if exists { return nil, nil, fmt.Errorf("duplicate assignment to `%s`, have: %s", uid, n) @@ -3000,11 +3035,27 @@ func (obj *StmtProg) Ordering(produces map[string]interfaces.Node) (*pgraph.Grap } prod[uid] = stmt // store } - if stmt, ok := x.(*StmtBind); ok { - if stmt.Ident == "" { - return nil, nil, fmt.Errorf("missing bind name") + + if stmt, ok := x.(*StmtClass); ok { + if stmt.Name == "" { + return nil, nil, fmt.Errorf("missing class name") } - uid := varOrderingPrefix + stmt.Ident // ordering id + uid := classOrderingPrefix + stmt.Name // ordering id + n, exists := prod[uid] + if exists { + return nil, nil, fmt.Errorf("duplicate assignment to `%s`, have: %s", uid, n) + } + prod[uid] = stmt // store + } + + if stmt, ok := x.(*StmtInclude); ok { + if stmt.Name == "" { + return nil, nil, fmt.Errorf("missing include name") + } + if stmt.Alias == "" { // not consumed + continue + } + uid := scopedOrderingPrefix + stmt.Alias // ordering id n, exists := prod[uid] if exists { return nil, nil, fmt.Errorf("duplicate assignment to `%s`, have: %s", uid, n) @@ -3683,8 +3734,12 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // un-consumed statements to be skipped. As a result, this simplifies // the graph significantly in cases of unused code, because they're not // given a chance to SetScope even though they're in the StmtProg list. + + // In the below loop which we iterate over in the correct scope order, + // we build up the scope (loopScope) as we go, so that subsequent uses + // of the scope include earlier definitions and scope additions. loopScope := newScope.Copy() - funcCount := make(map[string]int) + funcCount := make(map[string]int) // count the occurrences of a func for _, x := range nodeOrder { // these are in the correct order for SetScope stmt, ok := x.(interfaces.Stmt) if !ok { @@ -3848,7 +3903,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // TODO: do this in a deterministic (sorted) order for name, x := range includedScope.Variables { newName := alias + interfaces.ModuleSep + name - if alias == "*" { // XXX: not supported by parser atm! + if alias == interfaces.BareSymbol { // not supported by parser atm! + if !AllowBareIncludes { + return fmt.Errorf("bare includes disabled at compile time for include of `%s`", include.Name) + } newName = name } if previous, exists := newVariables[newName]; exists { @@ -3860,7 +3918,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } for name, x := range includedScope.Functions { newName := alias + interfaces.ModuleSep + name - if alias == "*" { // XXX: not supported by parser atm! + if alias == interfaces.BareSymbol { // not supported by parser atm! + if !AllowBareIncludes { + return fmt.Errorf("bare includes disabled at compile time for include of `%s`", include.Name) + } newName = name } if previous, exists := newFunctions[newName]; exists { @@ -3872,7 +3933,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } for name, x := range includedScope.Classes { newName := alias + interfaces.ModuleSep + name - if alias == "*" { // XXX: not supported by parser atm! + if alias == interfaces.BareSymbol { // not supported by parser atm! + if !AllowBareIncludes { + return fmt.Errorf("bare includes disabled at compile time for include of `%s`", include.Name) + } newName = name } if previous, exists := newClasses[newName]; exists { @@ -8470,15 +8534,31 @@ func (obj *ExprVar) Ordering(produces map[string]interfaces.Node) (*pgraph.Graph } uid := varOrderingPrefix + obj.Name // ordering id - cons := make(map[interfaces.Node]string) - cons[obj] = uid - node, exists := produces[uid] if exists { - edge := &pgraph.SimpleEdge{Name: "exprvar"} + edge := &pgraph.SimpleEdge{Name: "exprvar1"} graph.AddEdge(node, obj, edge) // prod -> cons } + // equivalent to: strings.Contains(obj.Name, interfaces.ModuleSep) + if split := strings.Split(obj.Name, interfaces.ModuleSep); len(split) > 1 { + // we contain a dot + uid = scopedOrderingPrefix + split[0] // just the first prefix + + // TODO: do we also want this second edge?? + node, exists := produces[uid] + if exists { + edge := &pgraph.SimpleEdge{Name: "exprvar2"} + graph.AddEdge(node, obj, edge) // prod -> cons + } + } + // It's okay to replace the normal `var` prefix, because we have the + // fancier `include` prefix which matches more generally... + + // TODO: we _can_ produce two uid's here, is it okay we only offer one? + cons := make(map[interfaces.Node]string) + cons[obj] = uid + return graph, cons, nil } From 768fe5d983fa023f64ea77a88a34d0e457696f9e Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 18:46:55 -0500 Subject: [PATCH 11/16] XXX: more including the better??? nodeOrder code This might be wrong actually... --- lang/ast/structs.go | 89 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 2448186c35..70513a0e34 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -143,7 +143,7 @@ const ( // scopedOrderingPrefix is a magic prefix used for the Ordering graph. // It is shared between imports and include as. - scopedOrderingPrefix = "prefix:" + scopedOrderingPrefix = "scoped:" // ErrNoStoredScope is an error that tells us we can't get a scope here. ErrNoStoredScope = interfaces.Error("scope is not stored in this node") @@ -3024,6 +3024,7 @@ func (obj *StmtProg) Ordering(produces map[string]interfaces.Node) (*pgraph.Grap } prod[uid] = stmt // store } + if stmt, ok := x.(*StmtFunc); ok { if stmt.Name == "" { return nil, nil, fmt.Errorf("missing func name") @@ -3665,18 +3666,46 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { orderingGraphSingleton = false } - nodeOrder, err := orderingGraph.TopologicalSort() + // Filter ordering graph before toposort! This prevents ambiguity + // between ordering strings in different scopes when it's not relevant. + allowStmts := make(map[interfaces.Stmt]struct{}) + for _, x := range obj.Body { + stmt, ok := x.(interfaces.Stmt) + if !ok { + continue + } + //if _, ok := x.(*StmtImport); ok { // TODO: should we skip this? + // continue + //} + allowStmts[stmt] = struct{}{} + } + filterFn := func(v pgraph.Vertex) (bool, error) { + stmt, ok := v.(interfaces.Stmt) + if !ok { + return false, nil // skip non statements + } + if _, exists := allowStmts[stmt]; !exists { + return false, nil // skip statements not in body + } + return true, nil + } + orderingGraphFiltered, err := orderingGraph.FilterGraphWithFn(filterFn) + if err != nil { + return errwrap.Wrapf(err, "could not filter ordering graph") + } + + nodeOrder, err := orderingGraphFiltered.TopologicalSort() if err != nil { // TODO: print the cycle in a prettier way (with names?) if obj.data.Debug { - obj.data.Logf("set scope: not a dag:\n%s", orderingGraph.Sprint()) + obj.data.Logf("set scope: not a dag:\n%s", orderingGraphFiltered.Sprint()) } return errwrap.Wrapf(err, "recursive reference while setting scope") } // XXX: implement ValidTopoSortOrder! //topoSanity := (RequireTopologicalOrdering || TopologicalOrderingWarning) - //if topoSanity && !orderingGraph.ValidTopoSortOrder(nodeOrder) { + //if topoSanity && !orderingGraphFiltered.ValidTopoSortOrder(nodeOrder) { // msg := "code is out of order, you're insane!" // if TopologicalOrderingWarning { // obj.data.Logf(msg) @@ -4593,18 +4622,34 @@ func (obj *StmtInclude) Ordering(produces map[string]interfaces.Node) (*pgraph.G if obj.Name == "" { return nil, nil, fmt.Errorf("missing class name") } - // TODO: do we want obj.Alias added in here? - uid := classOrderingPrefix + obj.Name // ordering id - cons := make(map[interfaces.Node]string) - cons[obj] = uid + uid := classOrderingPrefix + obj.Name // ordering id node, exists := produces[uid] if exists { - edge := &pgraph.SimpleEdge{Name: "stmtinclude"} + edge := &pgraph.SimpleEdge{Name: "stmtinclude1"} graph.AddEdge(node, obj, edge) // prod -> cons } + // equivalent to: strings.Contains(obj.Name, interfaces.ModuleSep) + if split := strings.Split(obj.Name, interfaces.ModuleSep); len(split) > 1 { + // we contain a dot + uid = scopedOrderingPrefix + split[0] // just the first prefix + + // TODO: do we also want this second edge?? + node, exists := produces[uid] + if exists { + edge := &pgraph.SimpleEdge{Name: "stmtinclude2"} + graph.AddEdge(node, obj, edge) // prod -> cons + } + } + // It's okay to replace the normal `class` prefix, because we have the + // fancier `scoped:` prefix which matches more generally... + + // TODO: we _can_ produce two uid's here, is it okay we only offer one? + cons := make(map[interfaces.Node]string) + cons[obj] = uid + for _, node := range obj.Args { g, c, err := node.Ordering(produces) if err != nil { @@ -7751,15 +7796,31 @@ func (obj *ExprCall) Ordering(produces map[string]interfaces.Node) (*pgraph.Grap uid = varOrderingPrefix + obj.Name // ordering id } - cons := make(map[interfaces.Node]string) - cons[obj] = uid - node, exists := produces[uid] if exists { - edge := &pgraph.SimpleEdge{Name: "exprcallname"} + edge := &pgraph.SimpleEdge{Name: "exprcallname1"} graph.AddEdge(node, obj, edge) // prod -> cons } + // equivalent to: strings.Contains(obj.Name, interfaces.ModuleSep) + if split := strings.Split(obj.Name, interfaces.ModuleSep); len(split) > 1 { + // we contain a dot + uid = scopedOrderingPrefix + split[0] // just the first prefix + + // TODO: do we also want this second edge?? + node, exists := produces[uid] + if exists { + edge := &pgraph.SimpleEdge{Name: "exprcallname2"} + graph.AddEdge(node, obj, edge) // prod -> cons + } + } + // It's okay to replace the normal `func` or `var` prefix, because we + // have the fancier `scoped:` prefix which matches more generally... + + // TODO: we _can_ produce two uid's here, is it okay we only offer one? + cons := make(map[interfaces.Node]string) + cons[obj] = uid + for _, node := range obj.Args { g, c, err := node.Ordering(produces) if err != nil { @@ -8553,7 +8614,7 @@ func (obj *ExprVar) Ordering(produces map[string]interfaces.Node) (*pgraph.Graph } } // It's okay to replace the normal `var` prefix, because we have the - // fancier `include` prefix which matches more generally... + // fancier `scoped:` prefix which matches more generally... // TODO: we _can_ produce two uid's here, is it okay we only offer one? cons := make(map[interfaces.Node]string) From 97553b41f506c01afefd0b32bcb934078f904126 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 19:03:18 -0500 Subject: [PATCH 12/16] XXX more misc hacking --- lang/ast/structs.go | 32 +++++++++++++------ .../TestAstFunc2/simple-scope-ordering3.txtar | 9 ++++++ .../TestAstFunc2/simple-scope-ordering4.txtar | 9 ++++++ 3 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 lang/interpret_test/TestAstFunc2/simple-scope-ordering3.txtar create mode 100644 lang/interpret_test/TestAstFunc2/simple-scope-ordering4.txtar diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 70513a0e34..5cac418881 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3563,6 +3563,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { newVariables := make(map[string]string) newFunctions := make(map[string]string) newClasses := make(map[string]string) + // TODO: If we added .Ordering() for *StmtImport, we could combine this + // loop with the main nodeOrder sorted topological ordering loop below! for _, x := range obj.Body { imp, ok := x.(*StmtImport) if !ok { @@ -3656,16 +3658,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { return errwrap.Wrapf(err, "could not generate ordering") } - // debugging visualizations - if obj.data.Debug && orderingGraphSingleton { - obj.data.Logf("running graphviz for ordering graph...") - if err := orderingGraph.ExecGraphviz("/tmp/graphviz-ordering.dot"); err != nil { - obj.data.Logf("graphviz: errored: %+v", err) - } - // Only generate the top-level one, to prevent overwriting this! - orderingGraphSingleton = false - } - // Filter ordering graph before toposort! This prevents ambiguity // between ordering strings in different scopes when it's not relevant. allowStmts := make(map[interfaces.Stmt]struct{}) @@ -3694,6 +3686,20 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { return errwrap.Wrapf(err, "could not filter ordering graph") } + // debugging visualizations + if obj.data.Debug && orderingGraphSingleton { + obj.data.Logf("running graphviz for ordering graph...") + if err := orderingGraph.ExecGraphviz("/tmp/graphviz-ordering.dot"); err != nil { + obj.data.Logf("graphviz: errored: %+v", err) + } + if err := orderingGraphFiltered.ExecGraphviz("/tmp/graphviz-ordering-filtered.dot"); err != nil { + obj.data.Logf("graphviz: errored: %+v", err) + } + // Only generate the top-level one, to prevent overwriting this! + orderingGraphSingleton = false + } + + nodeOrder, err := orderingGraphFiltered.TopologicalSort() if err != nil { // TODO: print the cycle in a prettier way (with names?) @@ -4918,6 +4924,12 @@ func (obj *StmtImport) Ordering(produces map[string]interfaces.Node) (*pgraph.Gr } graph.AddVertex(obj) + // Since we always run the imports before anything else in the StmtProg, + // we don't need to do anything special in here. + // TODO: If this statement is true, add this in so that imports can be + // done in the same iteration through StmtProg in SetScope with all of + // the other statements. + cons := make(map[interfaces.Node]string) return graph, cons, nil } diff --git a/lang/interpret_test/TestAstFunc2/simple-scope-ordering3.txtar b/lang/interpret_test/TestAstFunc2/simple-scope-ordering3.txtar new file mode 100644 index 0000000000..867dbd5b51 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/simple-scope-ordering3.txtar @@ -0,0 +1,9 @@ +-- main.mcl -- +# reverse order +test $i0.f0 {} +include c0 as i0 +class c0() { + $f0 = "hello" +} +-- OUTPUT -- +Vertex: test[hello] diff --git a/lang/interpret_test/TestAstFunc2/simple-scope-ordering4.txtar b/lang/interpret_test/TestAstFunc2/simple-scope-ordering4.txtar new file mode 100644 index 0000000000..2a9dfb464a --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/simple-scope-ordering4.txtar @@ -0,0 +1,9 @@ +-- main.mcl -- +# reverse order +test $i0.f0 {} +include c0("hello") as i0 +class c0($s) { + $f0 = $s +} +-- OUTPUT -- +Vertex: test[hello] From dc657e23efa553a3e8c9dcd14110889d8b2cfef0 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 19:05:53 -0500 Subject: [PATCH 13/16] XXX don't use the bad ordering code for now... --- lang/ast/structs.go | 62 +++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 5cac418881..4da29ee83c 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3660,31 +3660,31 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // Filter ordering graph before toposort! This prevents ambiguity // between ordering strings in different scopes when it's not relevant. - allowStmts := make(map[interfaces.Stmt]struct{}) - for _, x := range obj.Body { - stmt, ok := x.(interfaces.Stmt) - if !ok { - continue - } - //if _, ok := x.(*StmtImport); ok { // TODO: should we skip this? - // continue - //} - allowStmts[stmt] = struct{}{} - } - filterFn := func(v pgraph.Vertex) (bool, error) { - stmt, ok := v.(interfaces.Stmt) - if !ok { - return false, nil // skip non statements - } - if _, exists := allowStmts[stmt]; !exists { - return false, nil // skip statements not in body - } - return true, nil - } - orderingGraphFiltered, err := orderingGraph.FilterGraphWithFn(filterFn) - if err != nil { - return errwrap.Wrapf(err, "could not filter ordering graph") - } + //allowStmts := make(map[interfaces.Stmt]struct{}) + //for _, x := range obj.Body { + // stmt, ok := x.(interfaces.Stmt) + // if !ok { + // continue + // } + // //if _, ok := x.(*StmtImport); ok { // TODO: should we skip this? + // // continue + // //} + // allowStmts[stmt] = struct{}{} + //} + //filterFn := func(v pgraph.Vertex) (bool, error) { + // stmt, ok := v.(interfaces.Stmt) + // if !ok { + // return false, nil // skip non statements + // } + // if _, exists := allowStmts[stmt]; !exists { + // return false, nil // skip statements not in body + // } + // return true, nil + //} + //orderingGraphFiltered, err := orderingGraph.FilterGraphWithFn(filterFn) + //if err != nil { + // return errwrap.Wrapf(err, "could not filter ordering graph") + //} // debugging visualizations if obj.data.Debug && orderingGraphSingleton { @@ -3692,19 +3692,21 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { if err := orderingGraph.ExecGraphviz("/tmp/graphviz-ordering.dot"); err != nil { obj.data.Logf("graphviz: errored: %+v", err) } - if err := orderingGraphFiltered.ExecGraphviz("/tmp/graphviz-ordering-filtered.dot"); err != nil { - obj.data.Logf("graphviz: errored: %+v", err) - } + //if err := orderingGraphFiltered.ExecGraphviz("/tmp/graphviz-ordering-filtered.dot"); err != nil { + // obj.data.Logf("graphviz: errored: %+v", err) + //} // Only generate the top-level one, to prevent overwriting this! orderingGraphSingleton = false } - nodeOrder, err := orderingGraphFiltered.TopologicalSort() + //nodeOrder, err := orderingGraphFiltered.TopologicalSort() + nodeOrder, err := orderingGraph.TopologicalSort() if err != nil { // TODO: print the cycle in a prettier way (with names?) if obj.data.Debug { - obj.data.Logf("set scope: not a dag:\n%s", orderingGraphFiltered.Sprint()) + obj.data.Logf("set scope: not a dag:\n%s", orderingGraph.Sprint()) + //obj.data.Logf("set scope: not a dag:\n%s", orderingGraphFiltered.Sprint()) } return errwrap.Wrapf(err, "recursive reference while setting scope") } From 771175b8a9d99310a1c5187fb8c97e2ee0df11c2 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Sun, 7 Jan 2024 19:21:57 -0500 Subject: [PATCH 14/16] XXX fixup some tests --- .../class-include-as-class0.txtar | 22 +++++++++++++++++++ .../class-include-as-class1.txtar | 0 .../class-include-as-class2.txtar | 0 .../class-include-as-func-scope0.txtar | 0 .../class-include-as-func-scope1.txtar | 0 .../class-include-as-func0.txtar | 0 .../class-include-as-func1.txtar | 0 .../class-include-as-func2.txtar | 0 .../class-include-as-lambda-scope0.txtar | 0 .../class-include-as-lambda-scope1.txtar | 0 .../class-include-as-lambda0.txtar | 0 .../class-include-as-lambda1.txtar | 0 .../class-include-as-lambda2.txtar | 4 ++-- .../class-include-as-vars0.txtar | 0 .../class-include-as-vars1.txtar | 0 .../class-include-as-vars2.txtar | 0 .../class-include-as-vars3.txtar | 0 .../TestAstFunc2/import-scope-classes1.txtar | 21 ++++++++++++++++++ .../TestAstFunc2/import-scope-classes2.txtar | 22 +++++++++++++++++++ .../TestAstFunc2/import-scope-classes3.txtar | 22 +++++++++++++++++++ .../TestAstFunc2/import-scope-classes4.txtar | 22 +++++++++++++++++++ .../TestAstFunc2/import-scope-vars1.txtar | 15 +++++++++++++ .../TestAstFunc2/import-scope-vars2.txtar | 14 ++++++++++++ .../class-include-as-class0.txtar | 21 ------------------ 24 files changed, 140 insertions(+), 23 deletions(-) create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-class0.txtar rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-class1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-class2.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-func-scope0.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-func-scope1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-func0.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-func1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-func2.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-lambda-scope0.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-lambda-scope1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-lambda0.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-lambda1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-lambda2.txtar (93%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-vars0.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-vars1.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-vars2.txtar (100%) rename lang/interpret_test/{TestAstFunc3 => TestAstFunc2}/class-include-as-vars3.txtar (100%) create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-vars1.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar delete mode 100644 lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-class0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-class0.txtar new file mode 100644 index 0000000000..20b37814a9 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-class0.txtar @@ -0,0 +1,22 @@ +-- main.mcl -- +class c1 { + test "t1" {} + $y = "hello" + class c0 { + test "t2" {} + $x = "goodbye" + } +} +include c1 as i1 # has $y +include i1.c0 as i0 # has $x ...and $y + +test $i0.x {} # ok +test $i0.y {} # sneaky! +test $i1.y {} # ok +panic($i0.x != "goodbye") +panic($i1.y != "hello") +-- OUTPUT -- +Vertex: test[goodbye] +Vertex: test[hello] +Vertex: test[t1] +Vertex: test[t2] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-class1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-class1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-class1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-class2.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-func-scope0.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-func-scope0.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-func-scope0.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-func-scope1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-func-scope1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-func-scope1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-func0.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-func0.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-func0.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-func1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-func1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-func1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-func2.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-func2.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-func2.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda-scope0.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope0.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-lambda-scope0.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda-scope1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-lambda-scope1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-lambda-scope1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda0.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-lambda0.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-lambda0.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-lambda1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-lambda1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar similarity index 93% rename from lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar index db47e88821..f992075c17 100644 --- a/lang/interpret_test/TestAstFunc3/class-include-as-lambda2.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar @@ -13,6 +13,7 @@ class c0($b) { "goodbye" } } + $f0 = "hey" } class c1($b) { test "t4" {} @@ -28,13 +29,12 @@ include c1(true) as i1 include c1(false) as i2 test $i1.x() {} +test $i1.i0.f0 {} test $i2.x() {} test $i1.i0.f0() {} # I think these should work directly too. Do we want them to? test $i2.i0.f0() {} -- OUTPUT -- Vertex: test[goodbye] -Vertex: test[goodbye] -Vertex: test[hello] Vertex: test[hello] Vertex: test[t1] Vertex: test[t2] diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-vars0.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-vars0.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-vars0.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-vars1.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-vars1.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-vars1.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-vars2.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-vars2.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-vars2.txtar diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar similarity index 100% rename from lang/interpret_test/TestAstFunc3/class-include-as-vars3.txtar rename to lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar new file mode 100644 index 0000000000..6d968029bf --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar @@ -0,0 +1,21 @@ +-- main.mcl -- + +class c1() { + $x = "i am x" +} + +class c2() { + include c1 as g1 + + #$y = "i am y" + $z = "i am y and " + $g1.x +} + +include c2 as f1 + +#test $f1.z {} # yep - +#test $f1.x {} # no - +#test $f1.g1.x {} # yep - + +-- OUTPUT -- +Vertex: test[this is x.mcl and this is y.mcl] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar new file mode 100644 index 0000000000..5ff26bb010 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar @@ -0,0 +1,22 @@ +-- main.mcl -- + +$x = "i am x" # i am now top-level + +class c2() { + #$y = "i am y" + $z = "i am y and " + $x + + $newx = $x +} + +include c2 as f1 + +#test $f1.z {} # ok - +#test $f1.x {} # ??? tricky - the correct answer is it should not work, but it might already be +#test $f1.newx {} # ok - + +# the really tricky case +#test $f1.g1.x {} # - + +-- OUTPUT -- +Vertex: test[this is x.mcl and this is y.mcl] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar new file mode 100644 index 0000000000..4f32025163 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar @@ -0,0 +1,22 @@ +-- main.mcl -- + +$x = "i am x" # i am now top-level + +class c2() { + #$y = "i am y" + $z = "i am y and " + $x + + $x = $x # is this allowed? it should be right? but.... --- it should not be allowed because $x will already be inside the scope as it gets copied in from the top-level. +} + +include c2 as f1 + +#test $f1.z {} # ok - +#test $f1.x {} # ??? tricky - the correct answer is it should not work, but it might already be +#test $f1.newx {} # ok - + +# the really tricky case +#test $f1.g1.x {} # - + +-- OUTPUT -- +Vertex: test[this is x.mcl and this is y.mcl] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar new file mode 100644 index 0000000000..8432350bb4 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar @@ -0,0 +1,22 @@ +-- main.mcl -- + +$x = "i am x" # i am now top-level + +class c2() { + #$y = "i am y" + $z = "i am y and " + $x + + $x = "hey" # shadow +} + +include c2 as f1 + +#test $f1.z {} # ok - +#test $f1.x {} # ok should be hey - +#test $f1.newx {} # ok - + +# the really tricky case +#test $f1.g1.x {} # - + +-- OUTPUT -- +Vertex: test[this is x.mcl and this is y.mcl] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-vars1.txtar b/lang/interpret_test/TestAstFunc2/import-scope-vars1.txtar new file mode 100644 index 0000000000..3b58162e2b --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-vars1.txtar @@ -0,0 +1,15 @@ +-- metadata.yaml -- +#files: "files/" # these are some extra files we can use (is the default) +-- main.mcl -- +import "y.mcl" as g +test $g.y {} # should work +#test $g.x {} # should fail +test $g.f.x {} # should maybe work +-- x.mcl -- +$x = "this is x.mcl" +-- y.mcl -- +import "x.mcl" as f +$y = $f.x + " and this is y.mcl" +-- OUTPUT -- +Vertex: test[this is x.mcl] +Vertex: test[this is x.mcl and this is y.mcl] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar b/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar new file mode 100644 index 0000000000..ba3e0df689 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar @@ -0,0 +1,14 @@ +-- metadata.yaml -- +#files: "files/" # these are some extra files we can use (is the default) +-- main.mcl -- +import "y.mcl" as g +test $g.y {} # should work +test $g.x {} # should fail +test $g.f.x {} # should maybe work +-- x.mcl -- +$x = "this is x.mcl" +-- y.mcl -- +import "x.mcl" as f +$y = $f.x + " and this is y.mcl" +-- OUTPUT -- +# err: XXX diff --git a/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar b/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar deleted file mode 100644 index af514a53fa..0000000000 --- a/lang/interpret_test/TestAstFunc3/class-include-as-class0.txtar +++ /dev/null @@ -1,21 +0,0 @@ --- main.mcl -- -class c1 { - test "t1" {} - $x = "hello" - class c0 { - test "t2" {} - $y = "goodbye" - } -} -include c1 as i1 -include i1.c0 as i0 - -test $i0.x {} -test $i1.y {} # XXX sam says it's wrong should not exist -panic($i0.x != "hello") -panic($i1.y != "goodbye") --- OUTPUT -- -Vertex: test[goodbye] -Vertex: test[hello] -Vertex: test[t1] -Vertex: test[t2] From 3d795cdad0ef114944a853e5882f85c3e0f9f237 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 12 Jan 2024 13:54:25 -0500 Subject: [PATCH 15/16] XXX fixup more tests --- .../class-include-as-class2.txtar | 4 ++-- .../class-include-as-duplicate.txtar | 10 --------- .../class-include-as-duplicate0.txtar | 11 ++++++++++ .../class-include-as-duplicate1.txtar | 11 ++++++++++ .../class-include-as-duplicate2.txtar | 10 +++++++++ .../class-include-as-import1.txtar | 2 +- .../class-include-as-import2.txtar | 2 +- .../class-include-as-lambda2.txtar | 4 ++-- .../TestAstFunc2/class-include-as-vars3.txtar | 2 +- .../TestAstFunc2/import-scope-classes1.txtar | 9 ++++---- .../TestAstFunc2/import-scope-classes2.txtar | 18 +++++++-------- .../TestAstFunc2/import-scope-classes3.txtar | 20 ++++++++--------- .../TestAstFunc2/import-scope-classes4.txtar | 22 ++++++++++--------- .../TestAstFunc2/import-scope-classes5.txtar | 20 +++++++++++++++++ .../TestAstFunc2/import-scope-classes6.txtar | 22 +++++++++++++++++++ .../import-scope-vars2-fail.txtar | 14 ++++++++++++ .../TestAstFunc2/import-scope-vars2.txtar | 5 +++-- 17 files changed, 132 insertions(+), 54 deletions(-) delete mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-duplicate0.txtar create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-duplicate1.txtar create mode 100644 lang/interpret_test/TestAstFunc2/class-include-as-duplicate2.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes5.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-classes6.txtar create mode 100644 lang/interpret_test/TestAstFunc2/import-scope-vars2-fail.txtar diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar index edff7e84ff..156b8e7466 100644 --- a/lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-class2.txtar @@ -1,6 +1,6 @@ -- main.mcl -- class c1($b) { - if $b { # Scope doesn't leak up and out of `if` statement! + if $b { # scope doesn't leak up and out of `if` statement! class inner() { test "t1" {} } @@ -14,4 +14,4 @@ class c1($b) { include c1 as i1 include i1.inner -- OUTPUT -- -# err: XXX +# err: errSetScope: class `c1` expected 1 args but got 0 diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar deleted file mode 100644 index a74d35b4ed..0000000000 --- a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate.txtar +++ /dev/null @@ -1,10 +0,0 @@ --- main.mcl -- -class c1($s) { - $x = "got: ${s}" -} - -include c1("hey") as i1 -include c1("there") as ii -test $i1.x {} --- OUTPUT -- -# err: can't use the same `as`... diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate0.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate0.txtar new file mode 100644 index 0000000000..4fb80527b7 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate0.txtar @@ -0,0 +1,11 @@ +-- main.mcl -- +class c1() { + $x = "got: ${s}" +} + +# TODO: can this be allowed? +include c1 as i1 +include c1 as i1 +test $i1.x {} +-- OUTPUT -- +# err: errSetScope: could not generate ordering: duplicate assignment to `scoped:i1`, have: include(c1) diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate1.txtar new file mode 100644 index 0000000000..110971d421 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate1.txtar @@ -0,0 +1,11 @@ +-- main.mcl -- +class c1($s) { + $x = "got: ${s}" +} + +# TODO: can this be allowed? +include c1("hey") as i1 +include c1("hey") as i1 +test $i1.x {} +-- OUTPUT -- +# err: errSetScope: could not generate ordering: duplicate assignment to `scoped:i1`, have: include(c1) diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-duplicate2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate2.txtar new file mode 100644 index 0000000000..eaca6a9b7d --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/class-include-as-duplicate2.txtar @@ -0,0 +1,10 @@ +-- main.mcl -- +class c1($s) { + $x = "got: ${s}" +} + +include c1("hey") as i1 +include c1("there") as i1 +test $i1.x {} +-- OUTPUT -- +# err: errSetScope: could not generate ordering: duplicate assignment to `scoped:i1`, have: include(c1) diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar index a4dc51beff..85362af690 100644 --- a/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-import1.txtar @@ -7,4 +7,4 @@ class c1($s) { include c1("hey") as i1 test $i1.x {} -- OUTPUT -- -# err: can't use the same `as`... +# err: errSetScope: could not generate ordering: duplicate assignment to `scoped:i1`, have: import(fmt) diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar index 0be6010c14..a9500f813a 100644 --- a/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-import2.txtar @@ -7,4 +7,4 @@ class c1($s) { include c1("hey") as fmt test $fmt.x {} -- OUTPUT -- -# err: can't use the same `as`... +# err: errSetScope: could not generate ordering: duplicate assignment to `scoped:fmt`, have: import(fmt) diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar index f992075c17..bac1f9ba11 100644 --- a/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-lambda2.txtar @@ -13,7 +13,7 @@ class c0($b) { "goodbye" } } - $f0 = "hey" + #$f0 = "hey" } class c1($b) { test "t4" {} @@ -29,7 +29,7 @@ include c1(true) as i1 include c1(false) as i2 test $i1.x() {} -test $i1.i0.f0 {} +test $i1.i0.f0() {} test $i2.x() {} test $i1.i0.f0() {} # I think these should work directly too. Do we want them to? test $i2.i0.f0() {} diff --git a/lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar b/lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar index 076e873b7e..d51f1b4861 100644 --- a/lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar +++ b/lang/interpret_test/TestAstFunc2/class-include-as-vars3.txtar @@ -1,5 +1,5 @@ -- main.mcl -- -#$wat = "bad1" +$wat = "bad1" class c1($wat) { test $wat {} } diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar index 6d968029bf..b648a751b9 100644 --- a/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes1.txtar @@ -13,9 +13,10 @@ class c2() { include c2 as f1 -#test $f1.z {} # yep - -#test $f1.x {} # no - -#test $f1.g1.x {} # yep - +test $f1.z {} # yep +#test $f1.x {} # no +test $f1.g1.x {} # yep -- OUTPUT -- -Vertex: test[this is x.mcl and this is y.mcl] +Vertex: test[i am y and i am x] +Vertex: test[i am x] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar index 5ff26bb010..c1c626f54e 100644 --- a/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes2.txtar @@ -1,22 +1,20 @@ -- main.mcl -- - -$x = "i am x" # i am now top-level +$x = "i am x" # i am top-level class c2() { #$y = "i am y" $z = "i am y and " + $x - $newx = $x + $newx = $x + " and hello" } include c2 as f1 -#test $f1.z {} # ok - -#test $f1.x {} # ??? tricky - the correct answer is it should not work, but it might already be -#test $f1.newx {} # ok - - -# the really tricky case -#test $f1.g1.x {} # - +test $f1.z {} +test $f1.x {} # tricky +test $f1.newx {} -- OUTPUT -- -Vertex: test[this is x.mcl and this is y.mcl] +Vertex: test[i am x] +Vertex: test[i am x and hello] +Vertex: test[i am y and i am x] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar index 4f32025163..f1eb16f9e6 100644 --- a/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes3.txtar @@ -1,22 +1,20 @@ -- main.mcl -- - -$x = "i am x" # i am now top-level +$x = "i am x" # i am top-level class c2() { - #$y = "i am y" $z = "i am y and " + $x - $x = $x # is this allowed? it should be right? but.... --- it should not be allowed because $x will already be inside the scope as it gets copied in from the top-level. + # Since $x is pulled in from top-level automatically, we don't allow the + # re-definition by shadowing of the same variable. + $x = $x # not allowed + #$x = $x + "wow" # allowed? } include c2 as f1 -#test $f1.z {} # ok - -#test $f1.x {} # ??? tricky - the correct answer is it should not work, but it might already be -#test $f1.newx {} # ok - - -# the really tricky case -#test $f1.g1.x {} # - +test $f1.z {} +test $f1.x {} # tricky +test $f1.newx {} -- OUTPUT -- -Vertex: test[this is x.mcl and this is y.mcl] +# err: errSetScope: recursive reference while setting scope: not a dag diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar index 8432350bb4..3fd54e30e0 100644 --- a/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes4.txtar @@ -1,22 +1,24 @@ -- main.mcl -- - -$x = "i am x" # i am now top-level +$x1 = "i am x1" # i am top-level +$x2 = "i am x2" # i am top-level class c2() { - #$y = "i am y" - $z = "i am y and " + $x + $z = "i am y and " + $x1 - $x = "hey" # shadow + $x1 = "hey" # shadow } include c2 as f1 -#test $f1.z {} # ok - -#test $f1.x {} # ok should be hey - -#test $f1.newx {} # ok - +test $f1.z {} +test $f1.x1 {} # the really tricky case -#test $f1.g1.x {} # - +test $f1.x2 {} -- OUTPUT -- -Vertex: test[this is x.mcl and this is y.mcl] +XXX ORDERING BUG, SOMETIMES PASSES, SOMETIMES FAILS + +Vertex: test[hey] +Vertex: test[i am x2] +Vertex: test[i am y and hey] diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes5.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes5.txtar new file mode 100644 index 0000000000..ef4b4793d5 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes5.txtar @@ -0,0 +1,20 @@ +-- main.mcl -- +$x = "i am x" # i am top-level + +class c2() { + $z = "i am y and " + $x + + # Since $x is pulled in from top-level automatically, we don't allow the + # re-definition by shadowing of the same variable. + #$x = $x # not allowed + $x = $x + "wow" # allowed? +} + +include c2 as f1 + +test $f1.z {} +test $f1.x {} # tricky +test $f1.newx {} + +-- OUTPUT -- +XXX Should be allowed? diff --git a/lang/interpret_test/TestAstFunc2/import-scope-classes6.txtar b/lang/interpret_test/TestAstFunc2/import-scope-classes6.txtar new file mode 100644 index 0000000000..2ac3d5164f --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-classes6.txtar @@ -0,0 +1,22 @@ +-- main.mcl -- +$x = "i am x" # i am top-level + +class c2() { + $z = "i am y and " + $x + + # Since $x is pulled in from top-level automatically, we don't allow the + # re-definition by shadowing of the same variable. + #$x = $x # not allowed + $tmpx = $x + #$x = $x + "wow" # allowed? + $x = $tmpx + "wow" # allowed! +} + +include c2 as f1 + +test $f1.z {} +test $f1.x {} # tricky +test $f1.newx {} + +-- OUTPUT -- +XXX Should be allowed! diff --git a/lang/interpret_test/TestAstFunc2/import-scope-vars2-fail.txtar b/lang/interpret_test/TestAstFunc2/import-scope-vars2-fail.txtar new file mode 100644 index 0000000000..f543c45bd5 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/import-scope-vars2-fail.txtar @@ -0,0 +1,14 @@ +-- metadata.yaml -- +#files: "files/" # these are some extra files we can use (is the default) +-- main.mcl -- +import "y.mcl" as g +#test $g.y {} # should work +test $g.x {} # should fail +test $g.f.x {} # should maybe work +-- x.mcl -- +$x = "this is x.mcl" +-- y.mcl -- +import "x.mcl" as f +$y = $f.x + " and this is y.mcl" +-- OUTPUT -- +# err: errSetScope: variable g.x not in scope diff --git a/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar b/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar index ba3e0df689..cb60b5d006 100644 --- a/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar +++ b/lang/interpret_test/TestAstFunc2/import-scope-vars2.txtar @@ -3,7 +3,7 @@ -- main.mcl -- import "y.mcl" as g test $g.y {} # should work -test $g.x {} # should fail +#test $g.x {} # should fail test $g.f.x {} # should maybe work -- x.mcl -- $x = "this is x.mcl" @@ -11,4 +11,5 @@ $x = "this is x.mcl" import "x.mcl" as f $y = $f.x + " and this is y.mcl" -- OUTPUT -- -# err: XXX +Vertex: test[this is x.mcl and this is y.mcl] +Vertex: test[this is x.mcl] From cadcd40cba52730d25f9567d5ca28ef950d5f776 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 12 Jan 2024 14:11:21 -0500 Subject: [PATCH 16/16] XXX: misc cleanups --- lang/ast/structs.go | 51 +++++++++++---------------------------------- 1 file changed, 12 insertions(+), 39 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index 4da29ee83c..f52998d079 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -3658,34 +3658,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { return errwrap.Wrapf(err, "could not generate ordering") } - // Filter ordering graph before toposort! This prevents ambiguity - // between ordering strings in different scopes when it's not relevant. - //allowStmts := make(map[interfaces.Stmt]struct{}) - //for _, x := range obj.Body { - // stmt, ok := x.(interfaces.Stmt) - // if !ok { - // continue - // } - // //if _, ok := x.(*StmtImport); ok { // TODO: should we skip this? - // // continue - // //} - // allowStmts[stmt] = struct{}{} - //} - //filterFn := func(v pgraph.Vertex) (bool, error) { - // stmt, ok := v.(interfaces.Stmt) - // if !ok { - // return false, nil // skip non statements - // } - // if _, exists := allowStmts[stmt]; !exists { - // return false, nil // skip statements not in body - // } - // return true, nil - //} - //orderingGraphFiltered, err := orderingGraph.FilterGraphWithFn(filterFn) - //if err != nil { - // return errwrap.Wrapf(err, "could not filter ordering graph") - //} - // debugging visualizations if obj.data.Debug && orderingGraphSingleton { obj.data.Logf("running graphviz for ordering graph...") @@ -3699,7 +3671,6 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { orderingGraphSingleton = false } - //nodeOrder, err := orderingGraphFiltered.TopologicalSort() nodeOrder, err := orderingGraph.TopologicalSort() if err != nil { @@ -3777,7 +3748,7 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // of the scope include earlier definitions and scope additions. loopScope := newScope.Copy() funcCount := make(map[string]int) // count the occurrences of a func - for _, x := range nodeOrder { // these are in the correct order for SetScope + for _, x := range nodeOrder { // these are in the correct order for SetScope stmt, ok := x.(interfaces.Stmt) if !ok { continue @@ -3879,8 +3850,9 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { // now collect any include contents if include, ok := x.(*StmtInclude); ok { - // XXX: actually we don't want to check for duplicates, that's allowed... What about include foo as bar twice? - // XXX: or what about `include foo(true) as bar; include foo(false) as bar` ? + // We actually don't want to check for duplicates, that + // is allowed, if we `include foo as bar` twice it will + // currently not work, but if possible, we can allow it. // check for duplicates *in this scope* //if _, exists := includes[include.Name]; exists { // return fmt.Errorf("include `%s` already exists in this scope", include.Name) @@ -3897,11 +3869,12 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { continue // there isn't anything to do here } - // XXX: deal with alias duplicates and * includes and so on... - //if _, exists := aliases[alias]; exists { - // TODO: track separately to give a better error message here - // return fmt.Errorf("import/include alias `%s` already exists in this scope", alias) - //} + // NOTE: This gets caught in ordering instead of here... + // deal with alias duplicates and * includes and so on... + if _, exists := aliases[alias]; exists { + // TODO: track separately to give a better error message here + return fmt.Errorf("import/include alias `%s` already exists in this scope", alias) + } if include.class == nil { // programming error @@ -3985,8 +3958,8 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error { } // everything has been merged, move on to next include... - //includes[include.Name] = struct{}{} // mark as found in scope - //aliases[alias] = struct{}{} + //includes[include.Name] = struct{}{} // don't mark as found in scope + aliases[alias] = struct{}{} // do track these as a bonus } }