From 1e62c68bfe935ccfa5096880e63222ca404ba413 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Wed, 12 Jun 2024 16:26:54 -0700 Subject: [PATCH] Separate blueprint parsing and evaluating Before this cl, blueprint expressions were evaluated as they were parsed. We want to add a feature to select statements where we can bind the value of soome value from soong into a blueprint variable, that then can be used like a regular variable in the .bp file. This means that select statements need to hold whole unevalated expression trees, and have the ability to evaluate them later on when the value of the bound variable is known. This cl doesn't implement the new select syntax, but it does split blueprint's parsing and evaluating into two separate stages. We also store expressions in selects and evaluate them when the select is resolved. I didn't do extensive performance evaluation, but a simple comparison of the time of `touch Android.bp && m nothing` before/after this cl showed a 1 second speedup. (That was probably just noise) Bug: 323382414 Test: m nothing --no-skip-soong-tests Change-Id: I12f373719991afeb4aec76517153f32229d97ff2 --- bpfmt/bpfmt.go | 2 +- bpmodify/bpmodify.go | 20 +- context.go | 28 +-- module_ctx.go | 3 +- parser/ast.go | 343 ++++++++++++++++++++++++-------- parser/modify.go | 17 +- parser/parser.go | 372 +++++++++++----------------------- parser/parser_test.go | 409 ++------------------------------------ parser/printer.go | 6 +- parser/printer_test.go | 2 +- parser/sort.go | 4 +- proptools/configurable.go | 248 +++++++++++++++++------ proptools/extend.go | 28 +-- proptools/extend_test.go | 62 ++++-- proptools/unpack.go | 89 ++------- proptools/unpack_test.go | 154 ++++++++++++-- 16 files changed, 823 insertions(+), 964 deletions(-) diff --git a/bpfmt/bpfmt.go b/bpfmt/bpfmt.go index df8b87a..e78252d 100644 --- a/bpfmt/bpfmt.go +++ b/bpfmt/bpfmt.go @@ -66,7 +66,7 @@ func processReader(filename string, in io.Reader, out io.Writer) error { r := bytes.NewBuffer(src) - file, errs := parser.Parse(filename, r, parser.NewScope(nil)) + file, errs := parser.Parse(filename, r) if len(errs) > 0 { for _, err := range errs { fmt.Fprintln(os.Stderr, err) diff --git a/bpmodify/bpmodify.go b/bpmodify/bpmodify.go index 1df808e..98b1bee 100644 --- a/bpmodify/bpmodify.go +++ b/bpmodify/bpmodify.go @@ -85,7 +85,7 @@ func processFile(filename string, in io.Reader, out io.Writer) error { return err } r := bytes.NewBuffer(src) - file, errs := parser.Parse(filename, r, parser.NewScope(nil)) + file, errs := parser.Parse(filename, r) if len(errs) > 0 { for _, err := range errs { fmt.Fprintln(os.Stderr, err) @@ -131,11 +131,13 @@ func findModules(file *parser.File) (modified bool, errs []error) { for _, def := range file.Defs { if module, ok := def.(*parser.Module); ok { for _, prop := range module.Properties { - if prop.Name == "name" && prop.Value.Type() == parser.StringType && targetedModule(prop.Value.Eval().(*parser.String).Value) { - for _, p := range targetedProperties.properties { - m, newErrs := processModuleProperty(module, prop.Name, file, p) - errs = append(errs, newErrs...) - modified = modified || m + if prop.Name == "name" { + if stringValue, ok := prop.Value.(*parser.String); ok && targetedModule(stringValue.Value) { + for _, p := range targetedProperties.properties { + m, newErrs := processModuleProperty(module, prop.Name, file, p) + errs = append(errs, newErrs...) + modified = modified || m + } } } } @@ -194,7 +196,7 @@ func getOrCreateRecursiveProperty(module *parser.Module, name string, prefixes [ m := &module.Map for i, prefix := range prefixes { if prop, found := m.GetProperty(prefix); found { - if mm, ok := prop.Value.Eval().(*parser.Map); ok { + if mm, ok := prop.Value.(*parser.Map); ok { m = mm } else { // We've found a property in the AST and such property is not of type @@ -236,9 +238,9 @@ func processParameter(value parser.Expression, paramName, moduleName string, } if (*replaceProperty).size() != 0 { - if list, ok := value.Eval().(*parser.List); ok { + if list, ok := value.(*parser.List); ok { return parser.ReplaceStringsInList(list, (*replaceProperty).oldNameToNewName), nil - } else if str, ok := value.Eval().(*parser.String); ok { + } else if str, ok := value.(*parser.String); ok { oldVal := str.Value replacementValue := (*replaceProperty).oldNameToNewName[oldVal] if replacementValue != "" { diff --git a/context.go b/context.go index 1591b3c..c07959f 100644 --- a/context.go +++ b/context.go @@ -1221,9 +1221,9 @@ func (c *Context) parseOne(rootDir, filename string, reader io.Reader, return nil, nil, []error{err} } - scope.Remove("subdirs") - scope.Remove("optional_subdirs") - scope.Remove("build") + scope.DontInherit("subdirs") + scope.DontInherit("optional_subdirs") + scope.DontInherit("build") file, errs = parser.ParseAndEval(filename, reader, scope) if len(errs) > 0 { for i, err := range errs { @@ -1357,10 +1357,10 @@ func (c *Context) findSubdirBlueprints(dir string, subdirs []string, subdirsPos } func getLocalStringListFromScope(scope *parser.Scope, v string) ([]string, scanner.Position, error) { - if assignment, local := scope.Get(v); assignment == nil || !local { + if assignment := scope.GetLocal(v); assignment == nil { return nil, scanner.Position{}, nil } else { - switch value := assignment.Value.Eval().(type) { + switch value := assignment.Value.(type) { case *parser.List: ret := make([]string, 0, len(value.Values)) @@ -1386,24 +1386,6 @@ func getLocalStringListFromScope(scope *parser.Scope, v string) ([]string, scann } } -func getStringFromScope(scope *parser.Scope, v string) (string, scanner.Position, error) { - if assignment, _ := scope.Get(v); assignment == nil { - return "", scanner.Position{}, nil - } else { - switch value := assignment.Value.Eval().(type) { - case *parser.String: - return value.Value, assignment.EqualsPos, nil - case *parser.Bool, *parser.List: - return "", scanner.Position{}, &BlueprintError{ - Err: fmt.Errorf("%q must be a string", v), - Pos: assignment.EqualsPos, - } - default: - panic(fmt.Errorf("unknown value type: %d", assignment.Value.Type())) - } - } -} - // Clones a build logic module by calling the factory method for its module type, and then cloning // property values. Any values stored in the module object that are not stored in properties // structs will be lost. diff --git a/module_ctx.go b/module_ctx.go index 920b74e..78a13f9 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -1392,8 +1392,7 @@ func runAndRemoveLoadHooks(ctx *Context, config interface{}, module *moduleInfo, // // The filename is only used for reporting errors. func CheckBlueprintSyntax(moduleFactories map[string]ModuleFactory, filename string, contents string) []error { - scope := parser.NewScope(nil) - file, errs := parser.Parse(filename, strings.NewReader(contents), scope) + file, errs := parser.Parse(filename, strings.NewReader(contents)) if len(errs) != 0 { return errs } diff --git a/parser/ast.go b/parser/ast.go index 7aea5e0..74754b9 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -16,6 +16,7 @@ package parser import ( "fmt" + "os" "strings" "text/scanner" ) @@ -40,14 +41,13 @@ type Assignment struct { Name string NamePos scanner.Position Value Expression - OrigValue Expression EqualsPos scanner.Position Assigner string Referenced bool } func (a *Assignment) String() string { - return fmt.Sprintf("%s@%s %s %s (%s) %t", a.Name, a.EqualsPos, a.Assigner, a.Value, a.OrigValue, a.Referenced) + return fmt.Sprintf("%s@%s %s %s %t", a.Name, a.EqualsPos, a.Assigner, a.Value, a.Referenced) } func (a *Assignment) Pos() scanner.Position { return a.NamePos } @@ -139,11 +139,19 @@ type Expression interface { // Copy returns a copy of the Expression that will not affect the original if mutated Copy() Expression String() string - // Type returns the underlying Type enum of the Expression if it were to be evaluated + // Type returns the underlying Type enum of the Expression if it were to be evaluated, if it's known. + // It's possible that the type isn't known, such as when a select statement with a late-bound variable + // is used. For that reason, Type() is mostly for use in error messages, not to make logic decisions + // off of. Type() Type - // Eval returns an expression that is fully evaluated to a simple type (List, Map, String, or - // Bool). It will return the same object for every call to Eval(). - Eval() Expression + // Eval returns an expression that is fully evaluated to a simple type (List, Map, String, + // Bool, or Select). It will return the origional expression if possible, or allocate a + // new one if modifications were necessary. + Eval(scope *Scope) (Expression, error) + // PrintfInto will substitute any %s's in string literals in the AST with the provided + // value. It will modify the AST in-place. This is used to implement soong config value + // variables, but should be removed when those have switched to selects. + PrintfInto(value string) error } // ExpressionsAreSame tells whether the two values are the same Expression. @@ -157,9 +165,6 @@ func ExpressionsAreSame(a Expression, b Expression) (equal bool, err error) { // TODO(jeffrygaston) once positions are removed from Expression structs, // remove this function and have callers use reflect.DeepEqual(a, b) func hackyExpressionsAreSame(a Expression, b Expression) (equal bool, err error) { - if a.Type() != b.Type() { - return false, nil - } left, err := hackyFingerprint(a) if err != nil { return false, nil @@ -173,7 +178,7 @@ func hackyExpressionsAreSame(a Expression, b Expression) (equal bool, err error) } func hackyFingerprint(expression Expression) (fingerprint []byte, err error) { - assignment := &Assignment{"a", noPos, expression, expression, noPos, "=", false} + assignment := &Assignment{"a", noPos, expression, noPos, "=", false} module := &File{} module.Defs = append(module.Defs, assignment) p := newPrinter(module) @@ -183,17 +188,19 @@ func hackyFingerprint(expression Expression) (fingerprint []byte, err error) { type Type int const ( - BoolType Type = iota + 1 + UnknownType Type = iota + BoolType StringType Int64Type ListType MapType - NotEvaluatedType UnsetType ) func (t Type) String() string { switch t { + case UnknownType: + return "unknown" case BoolType: return "bool" case StringType: @@ -204,12 +211,10 @@ func (t Type) String() string { return "list" case MapType: return "map" - case NotEvaluatedType: - return "notevaluated" case UnsetType: return "unset" default: - panic(fmt.Errorf("Unknown type %d", t)) + panic(fmt.Sprintf("Unknown type %d", t)) } } @@ -217,7 +222,6 @@ type Operator struct { Args [2]Expression Operator rune OperatorPos scanner.Position - Value Expression } func (x *Operator) Copy() Expression { @@ -227,26 +231,137 @@ func (x *Operator) Copy() Expression { return &ret } -func (x *Operator) Eval() Expression { - return x.Value.Eval() +func (x *Operator) Type() Type { + t1 := x.Args[0].Type() + t2 := x.Args[1].Type() + if t1 == UnknownType { + return t2 + } + if t2 == UnknownType { + return t1 + } + if t1 != t2 { + return UnknownType + } + return t1 } -func (x *Operator) Type() Type { - return x.Args[0].Type() +func (x *Operator) Eval(scope *Scope) (Expression, error) { + return evaluateOperator(scope, x.Operator, x.Args[0], x.Args[1]) +} + +func evaluateOperator(scope *Scope, operator rune, left, right Expression) (Expression, error) { + if operator != '+' { + return nil, fmt.Errorf("unknown operator %c", operator) + } + l, err := left.Eval(scope) + if err != nil { + return nil, err + } + r, err := right.Eval(scope) + if err != nil { + return nil, err + } + + if _, ok := l.(*Select); !ok { + if _, ok := r.(*Select); ok { + // Promote l to a select so we can add r to it + l = &Select{ + Cases: []*SelectCase{{ + Value: l, + }}, + } + } + } + + l = l.Copy() + + switch v := l.(type) { + case *String: + if _, ok := r.(*String); !ok { + fmt.Fprintf(os.Stderr, "not ok") + } + v.Value += r.(*String).Value + case *Int64: + v.Value += r.(*Int64).Value + v.Token = "" + case *List: + v.Values = append(v.Values, r.(*List).Values...) + case *Map: + var err error + v.Properties, err = addMaps(scope, v.Properties, r.(*Map).Properties) + if err != nil { + return nil, err + } + case *Select: + v.Append = r + default: + return nil, fmt.Errorf("operator %c not supported on %v", operator, v) + } + + return l, nil +} + +func addMaps(scope *Scope, map1, map2 []*Property) ([]*Property, error) { + ret := make([]*Property, 0, len(map1)) + + inMap1 := make(map[string]*Property) + inMap2 := make(map[string]*Property) + inBoth := make(map[string]*Property) + + for _, prop1 := range map1 { + inMap1[prop1.Name] = prop1 + } + + for _, prop2 := range map2 { + inMap2[prop2.Name] = prop2 + if _, ok := inMap1[prop2.Name]; ok { + inBoth[prop2.Name] = prop2 + } + } + + for _, prop1 := range map1 { + if prop2, ok := inBoth[prop1.Name]; ok { + var err error + newProp := *prop1 + newProp.Value, err = evaluateOperator(scope, '+', prop1.Value, prop2.Value) + if err != nil { + return nil, err + } + ret = append(ret, &newProp) + } else { + ret = append(ret, prop1) + } + } + + for _, prop2 := range map2 { + if _, ok := inBoth[prop2.Name]; !ok { + ret = append(ret, prop2) + } + } + + return ret, nil +} + +func (x *Operator) PrintfInto(value string) error { + if err := x.Args[0].PrintfInto(value); err != nil { + return err + } + return x.Args[1].PrintfInto(value) } func (x *Operator) Pos() scanner.Position { return x.Args[0].Pos() } func (x *Operator) End() scanner.Position { return x.Args[1].End() } func (x *Operator) String() string { - return fmt.Sprintf("(%s %c %s = %s)@%s", x.Args[0].String(), x.Operator, x.Args[1].String(), - x.Value, x.OperatorPos) + return fmt.Sprintf("(%s %c %s)@%s", x.Args[0].String(), x.Operator, x.Args[1].String(), + x.OperatorPos) } type Variable struct { Name string NamePos scanner.Position - Value Expression + Type_ Type } func (x *Variable) Pos() scanner.Position { return x.NamePos } @@ -257,15 +372,27 @@ func (x *Variable) Copy() Expression { return &ret } -func (x *Variable) Eval() Expression { - return x.Value.Eval() +func (x *Variable) Eval(scope *Scope) (Expression, error) { + if assignment := scope.Get(x.Name); assignment != nil { + assignment.Referenced = true + return assignment.Value, nil + } + return nil, fmt.Errorf("undefined variable %s", x.Name) +} + +func (x *Variable) PrintfInto(value string) error { + return nil } func (x *Variable) String() string { - return x.Name + " = " + x.Value.String() + return x.Name } -func (x *Variable) Type() Type { return x.Value.Type() } +func (x *Variable) Type() Type { + // Variables do not normally have a type associated with them, this is only + // filled out in the androidmk tool + return x.Type_ +} type Map struct { LBracePos scanner.Position @@ -285,8 +412,30 @@ func (x *Map) Copy() Expression { return &ret } -func (x *Map) Eval() Expression { - return x +func (x *Map) Eval(scope *Scope) (Expression, error) { + newProps := make([]*Property, len(x.Properties)) + for i, prop := range x.Properties { + newVal, err := prop.Value.Eval(scope) + if err != nil { + return nil, err + } + newProps[i] = &Property{ + Name: prop.Name, + NamePos: prop.NamePos, + ColonPos: prop.ColonPos, + Value: newVal, + } + } + return &Map{ + LBracePos: x.LBracePos, + RBracePos: x.RBracePos, + Properties: newProps, + }, nil +} + +func (x *Map) PrintfInto(value string) error { + // We should never reach this because selects cannot hold maps + panic("printfinto() is unsupported on maps") } func (x *Map) String() string { @@ -379,8 +528,29 @@ func (x *List) Copy() Expression { return &ret } -func (x *List) Eval() Expression { - return x +func (x *List) Eval(scope *Scope) (Expression, error) { + newValues := make([]Expression, len(x.Values)) + for i, val := range x.Values { + newVal, err := val.Eval(scope) + if err != nil { + return nil, err + } + newValues[i] = newVal + } + return &List{ + LBracePos: x.LBracePos, + RBracePos: x.RBracePos, + Values: newValues, + }, nil +} + +func (x *List) PrintfInto(value string) error { + for _, val := range x.Values { + if err := val.PrintfInto(value); err != nil { + return err + } + } + return nil } func (x *List) String() string { @@ -407,8 +577,26 @@ func (x *String) Copy() Expression { return &ret } -func (x *String) Eval() Expression { - return x +func (x *String) Eval(scope *Scope) (Expression, error) { + return x, nil +} + +func (x *String) PrintfInto(value string) error { + count := strings.Count(x.Value, "%") + if count == 0 { + return nil + } + + if count > 1 { + return fmt.Errorf("list/value variable properties only support a single '%%'") + } + + if !strings.Contains(x.Value, "%s") { + return fmt.Errorf("unsupported %% in value variable property") + } + + x.Value = fmt.Sprintf(x.Value, value) + return nil } func (x *String) String() string { @@ -433,8 +621,12 @@ func (x *Int64) Copy() Expression { return &ret } -func (x *Int64) Eval() Expression { - return x +func (x *Int64) Eval(scope *Scope) (Expression, error) { + return x, nil +} + +func (x *Int64) PrintfInto(value string) error { + return nil } func (x *Int64) String() string { @@ -459,8 +651,12 @@ func (x *Bool) Copy() Expression { return &ret } -func (x *Bool) Eval() Expression { - return x +func (x *Bool) Eval(scope *Scope) (Expression, error) { + return x, nil +} + +func (x *Bool) PrintfInto(value string) error { + return nil } func (x *Bool) String() string { @@ -542,29 +738,6 @@ func (c Comment) Text() string { return string(buf) } -type NotEvaluated struct { - Position scanner.Position -} - -func (n NotEvaluated) Copy() Expression { - return NotEvaluated{Position: n.Position} -} - -func (n NotEvaluated) String() string { - return "Not Evaluated" -} - -func (n NotEvaluated) Type() Type { - return NotEvaluatedType -} - -func (n NotEvaluated) Eval() Expression { - return NotEvaluated{Position: n.Position} -} - -func (n NotEvaluated) Pos() scanner.Position { return n.Position } -func (n NotEvaluated) End() scanner.Position { return n.Position } - func endPos(pos scanner.Position, n int) scanner.Position { pos.Offset += n pos.Column += n @@ -609,13 +782,13 @@ func (c *ConfigurableCondition) String() string { } type Select struct { - KeywordPos scanner.Position // the keyword "select" - Conditions []ConfigurableCondition - LBracePos scanner.Position - RBracePos scanner.Position - Cases []*SelectCase // the case statements - Append Expression - ExpressionType Type + Scope *Scope // scope used to evaluate the body of the select later on + KeywordPos scanner.Position // the keyword "select" + Conditions []ConfigurableCondition + LBracePos scanner.Position + RBracePos scanner.Position + Cases []*SelectCase // the case statements + Append Expression } func (s *Select) Pos() scanner.Position { return s.KeywordPos } @@ -633,8 +806,14 @@ func (s *Select) Copy() Expression { return &ret } -func (s *Select) Eval() Expression { - return s +func (s *Select) Eval(scope *Scope) (Expression, error) { + s.Scope = scope + return s, nil +} + +func (x *Select) PrintfInto(value string) error { + // PrintfInto will be handled at the Configurable object level + panic("Cannot call PrintfInto on a select expression") } func (s *Select) String() string { @@ -642,10 +821,10 @@ func (s *Select) String() string { } func (s *Select) Type() Type { - if s.ExpressionType == UnsetType && s.Append != nil { - return s.Append.Type() + if len(s.Cases) == 0 { + return UnsetType } - return s.ExpressionType + return UnknownType } type SelectCase struct { @@ -681,21 +860,25 @@ type UnsetProperty struct { Position scanner.Position } -func (n UnsetProperty) Copy() Expression { - return UnsetProperty{Position: n.Position} +func (n *UnsetProperty) Copy() Expression { + return &UnsetProperty{Position: n.Position} } -func (n UnsetProperty) String() string { +func (n *UnsetProperty) String() string { return "unset" } -func (n UnsetProperty) Type() Type { +func (n *UnsetProperty) Type() Type { return UnsetType } -func (n UnsetProperty) Eval() Expression { - return UnsetProperty{Position: n.Position} +func (n *UnsetProperty) Eval(scope *Scope) (Expression, error) { + return n, nil } -func (n UnsetProperty) Pos() scanner.Position { return n.Position } -func (n UnsetProperty) End() scanner.Position { return n.Position } +func (x *UnsetProperty) PrintfInto(value string) error { + return nil +} + +func (n *UnsetProperty) Pos() scanner.Position { return n.Position } +func (n *UnsetProperty) End() scanner.Position { return n.Position } diff --git a/parser/modify.go b/parser/modify.go index a28fbe6..952e618 100644 --- a/parser/modify.go +++ b/parser/modify.go @@ -23,13 +23,11 @@ import ( func AddStringToList(list *List, s string) (modified bool) { for _, v := range list.Values { - if v.Type() != StringType { - panic(fmt.Errorf("expected string in list, got %s", v.Type())) - } - if sv, ok := v.(*String); ok && sv.Value == s { // string already exists return false + } else if !ok { + panic(fmt.Errorf("expected string in list, got %s", v.Type())) } } @@ -43,13 +41,11 @@ func AddStringToList(list *List, s string) (modified bool) { func RemoveStringFromList(list *List, s string) (modified bool) { for i, v := range list.Values { - if v.Type() != StringType { - panic(fmt.Errorf("expected string in list, got %s", v.Type())) - } - if sv, ok := v.(*String); ok && sv.Value == s { list.Values = append(list.Values[:i], list.Values[i+1:]...) return true + } else if !ok { + panic(fmt.Errorf("expected string in list, got %s", v.Type())) } } @@ -59,9 +55,6 @@ func RemoveStringFromList(list *List, s string) (modified bool) { func ReplaceStringsInList(list *List, replacements map[string]string) (replaced bool) { modified := false for i, v := range list.Values { - if v.Type() != StringType { - panic(fmt.Errorf("expected string in list, got %s", v.Type())) - } if sv, ok := v.(*String); ok && replacements[sv.Value] != "" { pos := list.Values[i].Pos() list.Values[i] = &String{ @@ -69,6 +62,8 @@ func ReplaceStringsInList(list *List, replacements map[string]string) (replaced Value: replacements[sv.Value], } modified = true + } else if !ok { + panic(fmt.Errorf("expected string in list, got %s", v.Type())) } } return modified diff --git a/parser/parser.go b/parser/parser.go index 81dafc9..d2030b6 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -45,22 +45,6 @@ type File struct { Comments []*CommentGroup } -func (f *File) Pos() scanner.Position { - return scanner.Position{ - Filename: f.Name, - Line: 1, - Column: 1, - Offset: 0, - } -} - -func (f *File) End() scanner.Position { - if len(f.Defs) > 0 { - return f.Defs[len(f.Defs)-1].End() - } - return noPos -} - func parse(p *parser) (file *File, errs []error) { defer func() { if r := recover(); r != nil { @@ -87,22 +71,54 @@ func parse(p *parser) (file *File, errs []error) { } func ParseAndEval(filename string, r io.Reader, scope *Scope) (file *File, errs []error) { - p := newParser(r, scope) - p.eval = true - p.scanner.Filename = filename + file, errs = Parse(filename, r) + if len(errs) > 0 { + return nil, errs + } - return parse(p) + // evaluate all module properties + var newDefs []Definition + for _, def := range file.Defs { + switch d := def.(type) { + case *Module: + for _, prop := range d.Map.Properties { + newval, err := prop.Value.Eval(scope) + if err != nil { + return nil, []error{err} + } + switch newval.(type) { + case *String, *Bool, *Int64, *Select, *Map, *List: + // ok + default: + panic(fmt.Sprintf("Evaled but got %#v\n", newval)) + } + prop.Value = newval + } + newDefs = append(newDefs, d) + case *Assignment: + if err := scope.HandleAssignment(d); err != nil { + return nil, []error{err} + } + } + } + + // This is not strictly necessary, but removing the assignments from + // the result makes it clearer that this is an evaluated file. + // We could also consider adding a "EvaluatedFile" type to return. + file.Defs = newDefs + + return file, nil } -func Parse(filename string, r io.Reader, scope *Scope) (file *File, errs []error) { - p := newParser(r, scope) +func Parse(filename string, r io.Reader) (file *File, errs []error) { + p := newParser(r) p.scanner.Filename = filename return parse(p) } func ParseExpression(r io.Reader) (value Expression, errs []error) { - p := newParser(r, NewScope(nil)) + p := newParser(r) p.next() value = p.parseExpression() p.accept(scanner.EOF) @@ -114,14 +130,11 @@ type parser struct { scanner scanner.Scanner tok rune errors []error - scope *Scope comments []*CommentGroup - eval bool } -func newParser(r io.Reader, scope *Scope) *parser { +func newParser(r io.Reader) *parser { p := &parser{} - p.scope = scope p.scanner.Init(r) p.scanner.Error = func(sc *scanner.Scanner, msg string) { p.errorf(msg) @@ -234,34 +247,9 @@ func (p *parser) parseAssignment(name string, namePos scanner.Position, assignment.Name = name assignment.NamePos = namePos assignment.Value = value - assignment.OrigValue = value assignment.EqualsPos = pos assignment.Assigner = assigner - if p.scope != nil { - if assigner == "+=" { - if old, local := p.scope.Get(assignment.Name); old == nil { - p.errorf("modified non-existent variable %q with +=", assignment.Name) - } else if !local { - p.errorf("modified non-local variable %q with +=", assignment.Name) - } else if old.Referenced { - p.errorf("modified variable %q with += after referencing", assignment.Name) - } else { - val, err := p.evaluateOperator(old.Value, assignment.Value, '+', assignment.EqualsPos) - if err != nil { - p.error(err) - } else { - old.Value = val - } - } - } else { - err := p.scope.Add(assignment) - if err != nil { - p.error(err) - } - } - } - return } @@ -297,13 +285,7 @@ func (p *parser) parseModule(typ string, typPos scanner.Position) *Module { func (p *parser) parsePropertyList(isModule, compat bool) (properties []*Property) { for p.tok == scanner.Ident { - property := p.parseProperty(isModule, compat) - - // If a property is set to an empty select or a select where all branches are "unset", - // skip emitting the property entirely. - if property.Value.Type() != UnsetType { - properties = append(properties, property) - } + properties = append(properties, p.parseProperty(isModule, compat)) if p.tok != ',' { // There was no comma, so the list is done. @@ -363,115 +345,6 @@ func (p *parser) parseExpression() (value Expression) { } } -func (p *parser) evaluateOperator(value1, value2 Expression, operator rune, - pos scanner.Position) (Expression, error) { - - if value1.Type() == UnsetType { - return value2, nil - } - if value2.Type() == UnsetType { - return value1, nil - } - - value := value1 - - if p.eval { - e1 := value1.Eval() - e2 := value2.Eval() - if e1.Type() != e2.Type() { - return nil, fmt.Errorf("mismatched type in operator %c: %s != %s", operator, - e1.Type(), e2.Type()) - } - - if _, ok := e1.(*Select); !ok { - if _, ok := e2.(*Select); ok { - // Promote e1 to a select so we can add e2 to it - e1 = &Select{ - Cases: []*SelectCase{{ - Value: e1, - }}, - ExpressionType: e1.Type(), - } - } - } - - value = e1.Copy() - - switch operator { - case '+': - switch v := value.(type) { - case *String: - v.Value += e2.(*String).Value - case *Int64: - v.Value += e2.(*Int64).Value - v.Token = "" - case *List: - v.Values = append(v.Values, e2.(*List).Values...) - case *Map: - var err error - v.Properties, err = p.addMaps(v.Properties, e2.(*Map).Properties, pos) - if err != nil { - return nil, err - } - case *Select: - v.Append = e2 - default: - return nil, fmt.Errorf("operator %c not supported on type %s", operator, v.Type()) - } - default: - panic("unknown operator " + string(operator)) - } - } - - return &Operator{ - Args: [2]Expression{value1, value2}, - Operator: operator, - OperatorPos: pos, - Value: value, - }, nil -} - -func (p *parser) addMaps(map1, map2 []*Property, pos scanner.Position) ([]*Property, error) { - ret := make([]*Property, 0, len(map1)) - - inMap1 := make(map[string]*Property) - inMap2 := make(map[string]*Property) - inBoth := make(map[string]*Property) - - for _, prop1 := range map1 { - inMap1[prop1.Name] = prop1 - } - - for _, prop2 := range map2 { - inMap2[prop2.Name] = prop2 - if _, ok := inMap1[prop2.Name]; ok { - inBoth[prop2.Name] = prop2 - } - } - - for _, prop1 := range map1 { - if prop2, ok := inBoth[prop1.Name]; ok { - var err error - newProp := *prop1 - newProp.Value, err = p.evaluateOperator(prop1.Value, prop2.Value, '+', pos) - if err != nil { - return nil, err - } - ret = append(ret, &newProp) - } else { - ret = append(ret, prop1) - } - } - - for _, prop2 := range map2 { - if _, ok := inBoth[prop2.Name]; !ok { - ret = append(ret, prop2) - } - } - - return ret, nil -} - func (p *parser) parseOperator(value1 Expression) Expression { operator := p.tok pos := p.scanner.Position @@ -479,14 +352,11 @@ func (p *parser) parseOperator(value1 Expression) Expression { value2 := p.parseExpression() - value, err := p.evaluateOperator(value1, value2, operator, pos) - if err != nil { - p.error(err) - return nil + return &Operator{ + Args: [2]Expression{value1, value2}, + Operator: operator, + OperatorPos: pos, } - - return value - } func (p *parser) parseValue() (value Expression) { @@ -535,22 +405,9 @@ func (p *parser) parseVariable() Expression { var value Expression text := p.scanner.TokenText() - if p.eval { - if assignment, local := p.scope.Get(text); assignment == nil { - p.errorf("variable %q is not set", text) - } else { - if local { - assignment.Referenced = true - } - value = assignment.Value - } - } else { - value = &NotEvaluated{} - } value = &Variable{ Name: text, NamePos: p.scanner.Position, - Value: value, } p.accept(scanner.Ident) @@ -723,7 +580,7 @@ func (p *parser) parseSelect() Expression { return nil } if p.tok == scanner.Ident && p.scanner.TokenText() == "unset" { - c.Value = UnsetProperty{Position: p.scanner.Position} + c.Value = &UnsetProperty{Position: p.scanner.Position} p.accept(scanner.Ident) } else { hasNonUnsetValue = true @@ -798,21 +655,6 @@ func (p *parser) parseSelect() Expression { } } - ty := UnsetType - for _, c := range result.Cases { - otherTy := c.Value.Type() - // Any other type can override UnsetType - if ty == UnsetType { - ty = otherTy - } - if otherTy != UnsetType && otherTy != ty { - p.errorf("Found select statement with differing types %q and %q in its cases", ty.String(), otherTy.String()) - return nil - } - } - - result.ExpressionType = ty - result.RBracePos = p.scanner.Position if !p.accept('}') { return nil @@ -913,79 +755,107 @@ func (p *parser) parseMapValue() *Map { } type Scope struct { - vars map[string]*Assignment - inheritedVars map[string]*Assignment + vars map[string]*Assignment + preventInheriting map[string]bool + parentScope *Scope } func NewScope(s *Scope) *Scope { - newScope := &Scope{ - vars: make(map[string]*Assignment), - inheritedVars: make(map[string]*Assignment), + return &Scope{ + vars: make(map[string]*Assignment), + preventInheriting: make(map[string]bool), + parentScope: s, } - - if s != nil { - for k, v := range s.vars { - newScope.inheritedVars[k] = v - } - for k, v := range s.inheritedVars { - newScope.inheritedVars[k] = v - } - } - - return newScope } -func (s *Scope) Add(assignment *Assignment) error { - if old, ok := s.vars[assignment.Name]; ok { - return fmt.Errorf("variable already set, previous assignment: %s", old) +func (s *Scope) HandleAssignment(assignment *Assignment) error { + switch assignment.Assigner { + case "+=": + if !s.preventInheriting[assignment.Name] && s.parentScope.Get(assignment.Name) != nil { + return fmt.Errorf("modified non-local variable %q with +=", assignment.Name) + } + if old, ok := s.vars[assignment.Name]; !ok { + return fmt.Errorf("modified non-existent variable %q with +=", assignment.Name) + } else if old.Referenced { + return fmt.Errorf("modified variable %q with += after referencing", assignment.Name) + } else { + newValue, err := evaluateOperator(s, '+', old.Value, assignment.Value) + if err != nil { + return err + } + old.Value = newValue + } + case "=": + if old, ok := s.vars[assignment.Name]; ok { + return fmt.Errorf("variable already set, previous assignment: %s", old) + } + + if old := s.parentScope.Get(assignment.Name); old != nil && !s.preventInheriting[assignment.Name] { + return fmt.Errorf("variable already set in inherited scope, previous assignment: %s", old) + } + + if newValue, err := assignment.Value.Eval(s); err != nil { + return err + } else { + assignment.Value = newValue + } + s.vars[assignment.Name] = assignment + default: + return fmt.Errorf("Unknown assigner '%s'", assignment.Assigner) } - - if old, ok := s.inheritedVars[assignment.Name]; ok { - return fmt.Errorf("variable already set in inherited scope, previous assignment: %s", old) - } - - s.vars[assignment.Name] = assignment - return nil } -func (s *Scope) Remove(name string) { - delete(s.vars, name) - delete(s.inheritedVars, name) +func (s *Scope) Get(name string) *Assignment { + if s == nil { + return nil + } + if a, ok := s.vars[name]; ok { + return a + } + if s.preventInheriting[name] { + return nil + } + return s.parentScope.Get(name) } -func (s *Scope) Get(name string) (*Assignment, bool) { +func (s *Scope) GetLocal(name string) *Assignment { + if s == nil { + return nil + } if a, ok := s.vars[name]; ok { - return a, true + return a } + return nil +} - if a, ok := s.inheritedVars[name]; ok { - return a, false - } - - return nil, false +// DontInherit prevents this scope from inheriting the given variable from its +// parent scope. +func (s *Scope) DontInherit(name string) { + s.preventInheriting[name] = true } func (s *Scope) String() string { - vars := []string{} + var sb strings.Builder + s.stringInner(&sb) + return sb.String() +} - for k := range s.vars { - vars = append(vars, k) +func (s *Scope) stringInner(sb *strings.Builder) { + if s == nil { + return } - for k := range s.inheritedVars { + vars := make([]string, 0, len(s.vars)) + for k := range s.vars { vars = append(vars, k) } sort.Strings(vars) - ret := []string{} for _, v := range vars { - if assignment, ok := s.vars[v]; ok { - ret = append(ret, assignment.String()) - } else { - ret = append(ret, s.inheritedVars[v].String()) - } + sb.WriteString(s.vars[v].String()) + sb.WriteRune('\n') } - return strings.Join(ret, "\n") + s.parentScope.stringInner(sb) } diff --git a/parser/parser_test.go b/parser/parser_test.go index 9de69c0..f2d09b6 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -565,12 +565,7 @@ var validParseTestCases = []struct { LiteralPos: mkpos(9, 2, 9), Value: "stuff", }, - OrigValue: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "bar", @@ -579,21 +574,8 @@ var validParseTestCases = []struct { Value: &Variable{ Name: "foo", NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, }, - OrigValue: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "baz", @@ -602,155 +584,26 @@ var validParseTestCases = []struct { Value: &Operator{ OperatorPos: mkpos(41, 4, 13), Operator: '+', - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuffstuff", - }, Args: [2]Expression{ &Variable{ Name: "foo", NamePos: mkpos(37, 4, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, }, &Variable{ Name: "bar", NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, }, }, }, - OrigValue: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuffstuff", - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - }, - }, - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "boo", NamePos: mkpos(49, 5, 3), EqualsPos: mkpos(53, 5, 7), - Value: &Operator{ - Args: [2]Expression{ - &Variable{ - Name: "baz", - NamePos: mkpos(55, 5, 9), - Value: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuffstuff", - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - }, - }, - }, - }, - &Variable{ - Name: "foo", - NamePos: mkpos(68, 6, 10), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - }, - OperatorPos: mkpos(66, 6, 8), - Operator: '+', - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuffstuffstuff", - }, - }, - OrigValue: &Variable{ + Value: &Variable{ Name: "baz", NamePos: mkpos(55, 5, 9), - Value: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuffstuff", - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - }, - }, - }, }, Assigner: "=", }, @@ -761,18 +614,6 @@ var validParseTestCases = []struct { Value: &Variable{ Name: "foo", NamePos: mkpos(68, 6, 10), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, - }, - OrigValue: &Variable{ - Name: "foo", - NamePos: mkpos(68, 6, 10), - Value: &String{ - LiteralPos: mkpos(9, 2, 9), - Value: "stuff", - }, }, Assigner: "+=", }, @@ -791,10 +632,6 @@ var validParseTestCases = []struct { Value: &Operator{ OperatorPos: mkpos(12, 2, 12), Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: -3, - }, Args: [2]Expression{ &Int64{ LiteralPos: mkpos(9, 2, 9), @@ -804,10 +641,6 @@ var validParseTestCases = []struct { &Operator{ OperatorPos: mkpos(17, 2, 17), Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(14, 2, 14), - Value: 1, - }, Args: [2]Expression{ &Int64{ LiteralPos: mkpos(14, 2, 14), @@ -823,43 +656,7 @@ var validParseTestCases = []struct { }, }, }, - OrigValue: &Operator{ - OperatorPos: mkpos(12, 2, 12), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: -3, - }, - Args: [2]Expression{ - &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: -4, - Token: "-4", - }, - &Operator{ - OperatorPos: mkpos(17, 2, 17), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(14, 2, 14), - Value: 1, - }, - Args: [2]Expression{ - &Int64{ - LiteralPos: mkpos(14, 2, 14), - Value: -5, - Token: "-5", - }, - &Int64{ - LiteralPos: mkpos(19, 2, 19), - Value: 6, - Token: "6", - }, - }, - }, - }, - }, - Assigner: "=", - Referenced: false, + Assigner: "=", }, }, nil, @@ -882,13 +679,7 @@ var validParseTestCases = []struct { Value: 1000000, Token: "1000000", }, - OrigValue: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "bar", @@ -897,23 +688,8 @@ var validParseTestCases = []struct { Value: &Variable{ Name: "foo", NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, }, - OrigValue: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "baz", @@ -922,164 +698,26 @@ var validParseTestCases = []struct { Value: &Operator{ OperatorPos: mkpos(41, 4, 13), Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 2000000, - }, Args: [2]Expression{ &Variable{ Name: "foo", NamePos: mkpos(37, 4, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, }, &Variable{ Name: "bar", NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, }, }, }, - OrigValue: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 2000000, - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - }, - }, - }, - Assigner: "=", - Referenced: true, + Assigner: "=", }, &Assignment{ Name: "boo", NamePos: mkpos(49, 5, 3), EqualsPos: mkpos(53, 5, 7), - Value: &Operator{ - Args: [2]Expression{ - &Variable{ - Name: "baz", - NamePos: mkpos(55, 5, 9), - Value: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 2000000, - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - }, - }, - }, - }, - &Variable{ - Name: "foo", - NamePos: mkpos(68, 6, 10), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - }, - OperatorPos: mkpos(66, 6, 8), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 3000000, - }, - }, - OrigValue: &Variable{ + Value: &Variable{ Name: "baz", NamePos: mkpos(55, 5, 9), - Value: &Operator{ - OperatorPos: mkpos(41, 4, 13), - Operator: '+', - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 2000000, - }, - Args: [2]Expression{ - &Variable{ - Name: "foo", - NamePos: mkpos(37, 4, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - &Variable{ - Name: "bar", - NamePos: mkpos(43, 4, 15), - Value: &Variable{ - Name: "foo", - NamePos: mkpos(25, 3, 9), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - }, - }, - }, }, Assigner: "=", }, @@ -1090,20 +728,6 @@ var validParseTestCases = []struct { Value: &Variable{ Name: "foo", NamePos: mkpos(68, 6, 10), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, - }, - OrigValue: &Variable{ - Name: "foo", - NamePos: mkpos(68, 6, 10), - Value: &Int64{ - LiteralPos: mkpos(9, 2, 9), - Value: 1000000, - Token: "1000000", - }, }, Assigner: "+=", }, @@ -1171,7 +795,7 @@ func TestParseValidInput(t *testing.T) { for i, testCase := range validParseTestCases { t.Run(strconv.Itoa(i), func(t *testing.T) { r := bytes.NewBufferString(testCase.input) - file, errs := ParseAndEval("", r, NewScope(nil)) + file, errs := Parse("", r) if len(errs) != 0 { t.Errorf("test case: %s", testCase.input) t.Errorf("unexpected errors:") @@ -1284,7 +908,7 @@ func TestParserEndPos(t *testing.T) { r := bytes.NewBufferString(in) - file, errs := ParseAndEval("", r, NewScope(nil)) + file, errs := Parse("", r) if len(errs) != 0 { t.Errorf("unexpected errors:") for _, err := range errs { @@ -1318,9 +942,8 @@ func TestParserEndPos(t *testing.T) { func TestParserNotEvaluated(t *testing.T) { // When parsing without evaluation, create variables correctly - scope := NewScope(nil) input := "FOO=abc\n" - _, errs := Parse("", bytes.NewBufferString(input), scope) + file, errs := Parse("", bytes.NewBufferString(input)) if errs != nil { t.Errorf("unexpected errors:") for _, err := range errs { @@ -1328,11 +951,11 @@ func TestParserNotEvaluated(t *testing.T) { } t.FailNow() } - assignment, found := scope.Get("FOO") - if !found { + assignment, ok := file.Defs[0].(*Assignment) + if !ok || assignment.Name != "FOO" { t.Fatalf("Expected to find FOO after parsing %s", input) } - if s := assignment.String(); strings.Contains(s, "PANIC") { - t.Errorf("Attempt to print FOO returned %s", s) + if assignment.Value.String() != "abc" { + t.Errorf("Attempt to print FOO returned %s", assignment.Value.String()) } } diff --git a/parser/printer.go b/parser/printer.go index c3ecf96..4d7ae7d 100644 --- a/parser/printer.go +++ b/parser/printer.go @@ -99,7 +99,7 @@ func (p *printer) printAssignment(assignment *Assignment) { p.requestSpace() p.printToken(assignment.Assigner, assignment.EqualsPos) p.requestSpace() - p.printExpression(assignment.OrigValue) + p.printExpression(assignment.Value) p.requestNewline() } @@ -134,7 +134,7 @@ func (p *printer) printExpression(value Expression) { case *Select: p.printSelect(v) default: - panic(fmt.Errorf("bad property type: %s", value.Type())) + panic(fmt.Errorf("bad property type: %v", value)) } } @@ -222,7 +222,7 @@ func (p *printer) printSelect(s *Select) { } p.printToken(":", c.ColonPos) p.requestSpace() - if unset, ok := c.Value.(UnsetProperty); ok { + if unset, ok := c.Value.(*UnsetProperty); ok { p.printToken(unset.String(), unset.Pos()) } else { p.printExpression(c.Value) diff --git a/parser/printer_test.go b/parser/printer_test.go index 60568eb..7b057aa 100644 --- a/parser/printer_test.go +++ b/parser/printer_test.go @@ -744,7 +744,7 @@ func TestPrinter(t *testing.T) { expected := testCase.output[1:] r := bytes.NewBufferString(in) - file, errs := Parse("", r, NewScope(nil)) + file, errs := Parse("", r) if len(errs) != 0 { t.Errorf("test case: %s", in) t.Errorf("unexpected errors:") diff --git a/parser/sort.go b/parser/sort.go index 52ff9f5..1b8fd8c 100644 --- a/parser/sort.go +++ b/parser/sort.go @@ -282,8 +282,8 @@ func isListOfPrimitives(values []Expression) bool { if len(values) == 0 { return true } - switch values[0].Type() { - case BoolType, StringType, Int64Type: + switch values[0].(type) { + case *Bool, *String, *Int64: return true default: return false diff --git a/proptools/configurable.go b/proptools/configurable.go index 8f101b1..5ed7308 100644 --- a/proptools/configurable.go +++ b/proptools/configurable.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/google/blueprint/optional" + "github.com/google/blueprint/parser" ) // ConfigurableOptional is the same as ShallowOptional, but we use this separate @@ -282,27 +283,46 @@ func (p *ConfigurablePattern) matchesValueType(v ConfigurableValue) bool { // different configurable properties. type ConfigurableCase[T ConfigurableElements] struct { patterns []ConfigurablePattern - value *T + value parser.Expression } type configurableCaseReflection interface { - initialize(patterns []ConfigurablePattern, value interface{}) + initialize(patterns []ConfigurablePattern, value parser.Expression) } var _ configurableCaseReflection = &ConfigurableCase[string]{} func NewConfigurableCase[T ConfigurableElements](patterns []ConfigurablePattern, value *T) ConfigurableCase[T] { + var valueExpr parser.Expression + if value == nil { + valueExpr = &parser.UnsetProperty{} + } else { + switch v := any(value).(type) { + case *string: + valueExpr = &parser.String{Value: *v} + case *bool: + valueExpr = &parser.Bool{Value: *v} + case *[]string: + innerValues := make([]parser.Expression, 0, len(*v)) + for _, x := range *v { + innerValues = append(innerValues, &parser.String{Value: x}) + } + valueExpr = &parser.List{Values: innerValues} + default: + panic(fmt.Sprintf("should be unreachable due to the ConfigurableElements restriction: %#v", value)) + } + } // Clone the values so they can't be modified from soong patterns = slices.Clone(patterns) return ConfigurableCase[T]{ patterns: patterns, - value: copyConfiguredValuePtr(value), + value: valueExpr, } } -func (c *ConfigurableCase[T]) initialize(patterns []ConfigurablePattern, value interface{}) { +func (c *ConfigurableCase[T]) initialize(patterns []ConfigurablePattern, value parser.Expression) { c.patterns = patterns - c.value = value.(*T) + c.value = value } // for the given T, return the reflect.type of configurableCase[T] @@ -384,6 +404,7 @@ type configurableInner[T ConfigurableElements] struct { type singleConfigurable[T ConfigurableElements] struct { conditions []ConfigurableCondition cases []ConfigurableCase[T] + scope *parser.Scope } // Ignore the warning about the unused marker variable, it's used via reflection @@ -415,7 +436,7 @@ func (c *Configurable[T]) AppendSimpleValue(value T) { c.inner = &configurableInner[T]{ single: singleConfigurable[T]{ cases: []ConfigurableCase[T]{{ - value: &value, + value: configuredValueToExpression(value), }}, }, } @@ -472,7 +493,12 @@ func (c *singleConfigurable[T]) evaluateNonTransitive(propertyName string, evalu if len(c.cases) == 0 { return nil } else if len(c.cases) == 1 { - return c.cases[0].value + if result, err := expressionToConfiguredValue[T](c.cases[0].value, c.scope); err != nil { + evaluator.PropertyErrorf(propertyName, "%s", err.Error()) + return nil + } else { + return result + } } else { evaluator.PropertyErrorf(propertyName, "Expected 0 or 1 branches in an unconfigured select, found %d", len(c.cases)) return nil @@ -499,7 +525,12 @@ func (c *singleConfigurable[T]) evaluateNonTransitive(propertyName string, evalu } } if allMatch && !foundMatch { - result = case_.value + if r, err := expressionToConfiguredValue[T](case_.value, c.scope); err != nil { + evaluator.PropertyErrorf(propertyName, "%s", err.Error()) + return nil + } else { + result = r + } foundMatch = true } } @@ -579,18 +610,19 @@ type configurableReflection interface { // Same as configurableReflection, but since initialize needs to take a pointer // to a Configurable, it was broken out into a separate interface. type configurablePtrReflection interface { - initialize(propertyName string, conditions []ConfigurableCondition, cases any) + initialize(scope *parser.Scope, propertyName string, conditions []ConfigurableCondition, cases any) } var _ configurableReflection = Configurable[string]{} var _ configurablePtrReflection = &Configurable[string]{} -func (c *Configurable[T]) initialize(propertyName string, conditions []ConfigurableCondition, cases any) { +func (c *Configurable[T]) initialize(scope *parser.Scope, propertyName string, conditions []ConfigurableCondition, cases any) { c.propertyName = propertyName c.inner = &configurableInner[T]{ single: singleConfigurable[T]{ conditions: conditions, cases: cases.([]ConfigurableCase[T]), + scope: scope, }, } } @@ -650,7 +682,7 @@ func (c *configurableInner[T]) appendSimpleValue(value T) { c.next = &configurableInner[T]{ single: singleConfigurable[T]{ cases: []ConfigurableCase[T]{{ - value: &value, + value: configuredValueToExpression(value), }}, }, } @@ -678,41 +710,13 @@ func (c *singleConfigurable[T]) printfInto(value string) error { if c.value == nil { continue } - switch v := any(c.value).(type) { - case *string: - if err := printfIntoString(v, value); err != nil { - return err - } - case *[]string: - for i := range *v { - if err := printfIntoString(&((*v)[i]), value); err != nil { - return err - } - } + if err := c.value.PrintfInto(value); err != nil { + return err } } return nil } -func printfIntoString(s *string, configValue string) error { - count := strings.Count(*s, "%") - if count == 0 { - return nil - } - - if count > 1 { - return fmt.Errorf("list/value variable properties only support a single '%%'") - } - - if !strings.Contains(*s, "%s") { - return fmt.Errorf("unsupported %% in value variable property") - } - - *s = fmt.Sprintf(*s, configValue) - - return nil -} - func (c Configurable[T]) clone() any { return Configurable[T]{ propertyName: c.propertyName, @@ -755,6 +759,9 @@ func (c *singleConfigurable[T]) isEmpty() bool { return false } if len(c.cases) == 1 && c.cases[0].value != nil { + if _, ok := c.cases[0].value.(*parser.UnsetProperty); ok { + return true + } return false } return true @@ -774,7 +781,7 @@ func (c *singleConfigurable[T]) alwaysHasValue() bool { return false } for _, c := range c.cases { - if c.value == nil { + if _, isUnset := c.value.(*parser.UnsetProperty); isUnset || c.value == nil { return false } } @@ -785,17 +792,84 @@ func (c Configurable[T]) configuredType() reflect.Type { return reflect.TypeOf((*T)(nil)).Elem() } -func copyConfiguredValuePtr[T ConfigurableElements](t *T) *T { - if t == nil { - return nil +func expressionToConfiguredValue[T ConfigurableElements](expr parser.Expression, scope *parser.Scope) (*T, error) { + expr, err := expr.Eval(scope) + if err != nil { + return nil, err } - switch t2 := any(*t).(type) { - case []string: - result := any(slices.Clone(t2)).(T) - return &result + switch e := expr.(type) { + case *parser.UnsetProperty: + return nil, nil + case *parser.String: + if result, ok := any(&e.Value).(*T); ok { + return result, nil + } else { + return nil, fmt.Errorf("can't assign string value to %s property", configuredTypeToString[T]()) + } + case *parser.Bool: + if result, ok := any(&e.Value).(*T); ok { + return result, nil + } else { + return nil, fmt.Errorf("can't assign bool value to %s property", configuredTypeToString[T]()) + } + case *parser.List: + result := make([]string, 0, len(e.Values)) + for _, x := range e.Values { + if y, ok := x.(*parser.String); ok { + result = append(result, y.Value) + } else { + return nil, fmt.Errorf("expected list of strings but found list of %s", x.Type()) + } + } + if result, ok := any(&result).(*T); ok { + return result, nil + } else { + return nil, fmt.Errorf("can't assign list of strings to list of %s property", configuredTypeToString[T]()) + } default: - x := *t - return &x + // If the expression was not evaluated beforehand we could hit this error even when the types match, + // but that's an internal logic error. + return nil, fmt.Errorf("expected %s but found %s (%#v)", configuredTypeToString[T](), expr.Type().String(), expr) + } +} + +func configuredValueToExpression[T ConfigurableElements](value T) parser.Expression { + switch v := any(value).(type) { + case string: + return &parser.String{Value: v} + case bool: + return &parser.Bool{Value: v} + case []string: + values := make([]parser.Expression, 0, len(v)) + for _, x := range v { + values = append(values, &parser.String{Value: x}) + } + return &parser.List{Values: values} + default: + panic("unhandled type in configuredValueToExpression") + } +} + +func configuredTypeToString[T ConfigurableElements]() string { + var zero T + switch any(zero).(type) { + case string: + return "string" + case bool: + return "bool" + case []string: + return "list of strings" + default: + panic("should be unreachable") + } +} + +func copyConfiguredValue[T ConfigurableElements](t T) T { + switch t2 := any(t).(type) { + case []string: + return any(slices.Clone(t2)).(T) + default: + return t } } @@ -812,18 +886,72 @@ func configuredValuePtrToOptional[T ConfigurableElements](t *T) ConfigurableOpti } } -func copyConfiguredValue[T ConfigurableElements](t T) T { - switch t2 := any(t).(type) { - case []string: - return any(slices.Clone(t2)).(T) - default: - return t - } -} - // PrintfIntoConfigurable replaces %s occurrences in strings in Configurable properties // with the provided string value. It's intention is to support soong config value variables // on Configurable properties. func PrintfIntoConfigurable(c any, value string) error { return c.(configurableReflection).printfInto(value) } + +func promoteValueToConfigurable(origional reflect.Value) reflect.Value { + var expr parser.Expression + var kind reflect.Kind + if origional.Kind() == reflect.Pointer && origional.IsNil() { + expr = &parser.UnsetProperty{} + kind = origional.Type().Elem().Kind() + } else { + if origional.Kind() == reflect.Pointer { + origional = origional.Elem() + } + kind = origional.Kind() + switch kind { + case reflect.String: + expr = &parser.String{Value: origional.String()} + case reflect.Bool: + expr = &parser.Bool{Value: origional.Bool()} + case reflect.Slice: + strList := origional.Interface().([]string) + exprList := make([]parser.Expression, 0, len(strList)) + for _, x := range strList { + exprList = append(exprList, &parser.String{Value: x}) + } + expr = &parser.List{Values: exprList} + default: + panic("can only convert string/bool/[]string to configurable") + } + } + switch kind { + case reflect.String: + return reflect.ValueOf(Configurable[string]{ + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ + cases: []ConfigurableCase[string]{{ + value: expr, + }}, + }, + }, + }) + case reflect.Bool: + return reflect.ValueOf(Configurable[bool]{ + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: expr, + }}, + }, + }, + }) + case reflect.Slice: + return reflect.ValueOf(Configurable[[]string]{ + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + cases: []ConfigurableCase[[]string]{{ + value: expr, + }}, + }, + }, + }) + default: + panic(fmt.Sprintf("Can't convert %s property to a configurable", origional.Kind().String())) + } +} diff --git a/proptools/extend.go b/proptools/extend.go index ec25d51..1bcb725 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -473,33 +473,7 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { // structs when they want to change the default values of properties. srcFieldType := srcFieldValue.Type() if isConfigurable(dstFieldValue.Type()) && !isConfigurable(srcFieldType) { - var value reflect.Value - if srcFieldType.Kind() == reflect.Pointer { - srcFieldType = srcFieldType.Elem() - if srcFieldValue.IsNil() { - value = srcFieldValue - } else { - // Copy the pointer - value = reflect.New(srcFieldType) - value.Elem().Set(srcFieldValue.Elem()) - } - } else { - value = reflect.New(srcFieldType) - value.Elem().Set(srcFieldValue) - } - caseType := configurableCaseType(srcFieldType) - case_ := reflect.New(caseType) - case_.Interface().(configurableCaseReflection).initialize(nil, value.Interface()) - cases := reflect.MakeSlice(reflect.SliceOf(caseType), 0, 1) - cases = reflect.Append(cases, case_.Elem()) - ct, err := configurableType(srcFieldType) - if err != nil { - // Should be unreachable due to earlier checks - panic(err.Error()) - } - temp := reflect.New(ct) - temp.Interface().(configurablePtrReflection).initialize("", nil, cases.Interface()) - srcFieldValue = temp.Elem() + srcFieldValue = promoteValueToConfigurable(srcFieldValue) } switch srcFieldValue.Kind() { diff --git a/proptools/extend_test.go b/proptools/extend_test.go index 13fd04f..0148204 100644 --- a/proptools/extend_test.go +++ b/proptools/extend_test.go @@ -20,6 +20,8 @@ import ( "reflect" "strings" "testing" + + "github.com/google/blueprint/parser" ) type appendPropertyTestCase struct { @@ -1272,7 +1274,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: &[]string{"1", "2"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "1"}, + &parser.String{Value: "2"}, + }}, }}, }, }, @@ -1293,7 +1298,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "b", }}, - value: &[]string{"3", "4"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "3"}, + &parser.String{Value: "4"}, + }}, }}, }, }, @@ -1315,7 +1323,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: &[]string{"1", "2"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "1"}, + &parser.String{Value: "2"}, + }}, }}, }, next: &configurableInner[[]string]{ @@ -1331,7 +1342,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "b", }}, - value: &[]string{"3", "4"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "3"}, + &parser.String{Value: "4"}, + }}, }}, }, }, @@ -1358,7 +1372,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: &[]string{"1", "2"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "1"}, + &parser.String{Value: "2"}, + }}, }}, }, }, @@ -1379,7 +1396,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "b", }}, - value: &[]string{"3", "4"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "3"}, + &parser.String{Value: "4"}, + }}, }}, }, }, @@ -1400,7 +1420,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "b", }}, - value: &[]string{"3", "4"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "3"}, + &parser.String{Value: "4"}, + }}, }}, }, next: &configurableInner[[]string]{ @@ -1417,7 +1440,10 @@ func appendPropertiesTestCases() []appendPropertyTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: &[]string{"1", "2"}, + value: &parser.List{Values: []parser.Expression{ + &parser.String{Value: "1"}, + &parser.String{Value: "2"}, + }}, }}, }, }, @@ -1907,12 +1933,12 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: BoolPtr(false), + value: &parser.Bool{Value: false}, }}, }, }, @@ -1939,18 +1965,18 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: BoolPtr(false), + value: &parser.Bool{Value: false}, }}, }, next: &configurableInner[bool]{ single: singleConfigurable[bool]{ cases: []ConfigurableCase[bool]{{ - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }}, }, }, @@ -1979,12 +2005,12 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: BoolPtr(false), + value: &parser.Bool{Value: false}, }}, }, }, @@ -2011,18 +2037,18 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { typ: configurablePatternTypeString, stringValue: "a", }}, - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: BoolPtr(false), + value: &parser.Bool{Value: false}, }}, }, next: &configurableInner[bool]{ single: singleConfigurable[bool]{ cases: []ConfigurableCase[bool]{{ - value: BoolPtr(true), + value: &parser.Bool{Value: true}, }}, }, }, diff --git a/proptools/unpack.go b/proptools/unpack.go index 1b48a61..2e74cd1 100644 --- a/proptools/unpack.go +++ b/proptools/unpack.go @@ -158,7 +158,7 @@ func (ctx *unpackContext) buildPropertyMap(prefix string, properties []*parser.P } ctx.propertyMap[name] = &packedProperty{property, false} - switch propValue := property.Value.Eval().(type) { + switch propValue := property.Value.(type) { case *parser.Map: ctx.buildPropertyMap(name, propValue.Properties) case *parser.List: @@ -313,7 +313,7 @@ func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect. return } } else if isStruct(fieldValue.Type()) { - if property.Value.Eval().Type() != parser.MapType { + if property.Value.Type() != parser.MapType { ctx.addError(&UnpackError{ fmt.Errorf("can't assign %s value to map property %q", property.Value.Type(), property.Name), @@ -359,7 +359,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa inner: &configurableInner[string]{ single: singleConfigurable[string]{ cases: []ConfigurableCase[string]{{ - value: &v.Value, + value: v, }}, }, }, @@ -379,7 +379,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa inner: &configurableInner[bool]{ single: singleConfigurable[bool]{ cases: []ConfigurableCase[bool]{{ - value: &v.Value, + value: v, }}, }, }, @@ -416,7 +416,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa inner: &configurableInner[[]string]{ single: singleConfigurable[[]string]{ cases: []ConfigurableCase[[]string]{{ - value: &value, + value: v, }}, }, }, @@ -425,12 +425,6 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa default: panic("This should be unreachable because ConfigurableElements only accepts slices of strings") } - case *parser.Operator: - property.Value = v.Value.Eval() - return ctx.unpackToConfigurable(propertyName, property, configurableType, configuredType) - case *parser.Variable: - property.Value = v.Value.Eval() - return ctx.unpackToConfigurable(propertyName, property, configurableType, configuredType) case *parser.Select: resultPtr := reflect.New(configurableType) result := resultPtr.Elem() @@ -448,13 +442,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa configurableCaseType := configurableCaseType(configuredType) cases := reflect.MakeSlice(reflect.SliceOf(configurableCaseType), 0, len(v.Cases)) - for i, c := range v.Cases { - p := &parser.Property{ - Name: property.Name + "[" + strconv.Itoa(i) + "]", - NamePos: c.ColonPos, - Value: c.Value, - } - + for _, c := range v.Cases { patterns := make([]ConfigurablePattern, len(c.Patterns)) for i, pat := range c.Patterns { switch pat := pat.(type) { @@ -473,40 +461,12 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } } - var value reflect.Value - // Map the "unset" keyword to a nil pointer in the cases map - if _, ok := c.Value.(parser.UnsetProperty); ok { - value = reflect.Zero(reflect.PointerTo(configuredType)) - } else { - var err error - switch configuredType.Kind() { - case reflect.String, reflect.Bool: - value, err = propertyToValue(reflect.PointerTo(configuredType), p) - if err != nil { - ctx.addError(&UnpackError{ - err, - c.Value.Pos(), - }) - return reflect.New(configurableType), false - } - case reflect.Slice: - if configuredType.Elem().Kind() != reflect.String { - panic("This should be unreachable because ConfigurableElements only accepts slices of strings") - } - value, ok = ctx.unpackToSlice(p.Name, p, reflect.PointerTo(configuredType)) - if !ok { - return reflect.New(configurableType), false - } - default: - panic("This should be unreachable because ConfigurableElements only accepts strings, boools, or slices of strings") - } - } - case_ := reflect.New(configurableCaseType) - case_.Interface().(configurableCaseReflection).initialize(patterns, value.Interface()) + case_.Interface().(configurableCaseReflection).initialize(patterns, c.Value) cases = reflect.Append(cases, case_.Elem()) } resultPtr.Interface().(configurablePtrReflection).initialize( + v.Scope, property.Name, conditions, cases.Interface(), @@ -537,7 +497,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa // If the given property is a select, returns an error saying that you can't assign a select to // a non-configurable property. Otherwise returns nil. func selectOnNonConfigurablePropertyError(property *parser.Property) error { - if _, ok := property.Value.Eval().(*parser.Select); !ok { + if _, ok := property.Value.(*parser.Select); !ok { return nil } @@ -570,7 +530,7 @@ func (ctx *unpackContext) unpackToSlice( // does. func (ctx *unpackContext) unpackToSliceInner( sliceName string, property *parser.Property, sliceType reflect.Type) (reflect.Value, bool) { - propValueAsList, ok := property.Value.Eval().(*parser.List) + propValueAsList, ok := property.Value.(*parser.List) if !ok { if err := selectOnNonConfigurablePropertyError(property); err != nil { ctx.addError(err) @@ -590,33 +550,24 @@ func (ctx *unpackContext) unpackToSliceInner( } // The function to construct an item value depends on the type of list elements. - var getItemFunc func(*parser.Property, reflect.Type) (reflect.Value, bool) - switch exprs[0].Type() { - case parser.BoolType, parser.StringType, parser.Int64Type: - getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) { + getItemFunc := func(property *parser.Property, t reflect.Type) (reflect.Value, bool) { + switch property.Value.(type) { + case *parser.Bool, *parser.String, *parser.Int64: value, err := propertyToValue(t, property) if err != nil { ctx.addError(err) return value, false } return value, true - } - case parser.ListType: - getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) { + case *parser.List: return ctx.unpackToSlice(property.Name, property, t) - } - case parser.MapType: - getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) { + case *parser.Map: itemValue := reflect.New(t).Elem() ctx.unpackToStruct(property.Name, itemValue) return itemValue, true + default: + panic(fmt.Errorf("bizarre property expression type: %v, %#v", property.Value.Type(), property.Value)) } - case parser.NotEvaluatedType: - getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) { - return reflect.New(t), false - } - default: - panic(fmt.Errorf("bizarre property expression type: %v", exprs[0].Type())) } itemProperty := &parser.Property{NamePos: property.NamePos, ColonPos: property.ColonPos} @@ -657,7 +608,7 @@ func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value switch kind := baseType.Kind(); kind { case reflect.Bool: - b, ok := property.Value.Eval().(*parser.Bool) + b, ok := property.Value.(*parser.Bool) if !ok { if err := selectOnNonConfigurablePropertyError(property); err != nil { return value, err @@ -672,7 +623,7 @@ func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value value = reflect.ValueOf(b.Value) case reflect.Int64: - b, ok := property.Value.Eval().(*parser.Int64) + b, ok := property.Value.(*parser.Int64) if !ok { return value, &UnpackError{ fmt.Errorf("can't assign %s value to int64 property %q", @@ -683,7 +634,7 @@ func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value value = reflect.ValueOf(b.Value) case reflect.String: - s, ok := property.Value.Eval().(*parser.String) + s, ok := property.Value.(*parser.String) if !ok { if err := selectOnNonConfigurablePropertyError(property); err != nil { return value, err diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go index 5e333b6..3af79fa 100644 --- a/proptools/unpack_test.go +++ b/proptools/unpack_test.go @@ -18,6 +18,7 @@ import ( "bytes" "reflect" "testing" + "text/scanner" "github.com/google/blueprint/parser" ) @@ -737,7 +738,14 @@ var validUnpackTestCases = []struct { inner: &configurableInner[string]{ single: singleConfigurable[string]{ cases: []ConfigurableCase[string]{{ - value: StringPtr("bar"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 17, + Line: 3, + Column: 10, + }, + Value: "bar", + }, }}, }, }, @@ -761,7 +769,15 @@ var validUnpackTestCases = []struct { inner: &configurableInner[bool]{ single: singleConfigurable[bool]{ cases: []ConfigurableCase[bool]{{ - value: BoolPtr(true), + value: &parser.Bool{ + LiteralPos: scanner.Position{ + Offset: 17, + Line: 3, + Column: 10, + }, + Value: true, + Token: "true", + }, }}, }, }, @@ -785,7 +801,36 @@ var validUnpackTestCases = []struct { inner: &configurableInner[[]string]{ single: singleConfigurable[[]string]{ cases: []ConfigurableCase[[]string]{{ - value: &[]string{"a", "b"}, + value: &parser.List{ + LBracePos: scanner.Position{ + Offset: 17, + Line: 3, + Column: 10, + }, + RBracePos: scanner.Position{ + Offset: 26, + Line: 3, + Column: 19, + }, + Values: []parser.Expression{ + &parser.String{ + LiteralPos: scanner.Position{ + Offset: 18, + Line: 3, + Column: 11, + }, + Value: "a", + }, + &parser.String{ + LiteralPos: scanner.Position{ + Offset: 23, + Line: 3, + Column: 16, + }, + Value: "b", + }, + }, + }, }}, }, }, @@ -812,6 +857,7 @@ var validUnpackTestCases = []struct { propertyName: "foo", inner: &configurableInner[string]{ single: singleConfigurable[string]{ + scope: parser.NewScope(nil), conditions: []ConfigurableCondition{{ functionName: "soong_config_variable", args: []string{ @@ -825,20 +871,41 @@ var validUnpackTestCases = []struct { typ: configurablePatternTypeString, stringValue: "a", }}, - value: StringPtr("a2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 90, + Line: 4, + Column: 11, + }, + Value: "a2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, stringValue: "b", }}, - value: StringPtr("b2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 106, + Line: 5, + Column: 11, + }, + Value: "b2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: StringPtr("c2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 126, + Line: 6, + Column: 15, + }, + Value: "c2", + }, }, }, }, @@ -870,6 +937,7 @@ var validUnpackTestCases = []struct { propertyName: "foo", inner: &configurableInner[string]{ single: singleConfigurable[string]{ + scope: parser.NewScope(nil), conditions: []ConfigurableCondition{{ functionName: "soong_config_variable", args: []string{ @@ -883,25 +951,47 @@ var validUnpackTestCases = []struct { typ: configurablePatternTypeString, stringValue: "a", }}, - value: StringPtr("a2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 90, + Line: 4, + Column: 11, + }, + Value: "a2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, stringValue: "b", }}, - value: StringPtr("b2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 106, + Line: 5, + Column: 11, + }, + Value: "b2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: StringPtr("c2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 126, + Line: 6, + Column: 15, + }, + Value: "c2", + }, }, }, }, next: &configurableInner[string]{ single: singleConfigurable[string]{ + scope: parser.NewScope(nil), conditions: []ConfigurableCondition{{ functionName: "soong_config_variable", args: []string{ @@ -915,20 +1005,41 @@ var validUnpackTestCases = []struct { typ: configurablePatternTypeString, stringValue: "d", }}, - value: StringPtr("d2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 218, + Line: 8, + Column: 11, + }, + Value: "d2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, stringValue: "e", }}, - value: StringPtr("e2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 234, + Line: 9, + Column: 11, + }, + Value: "e2", + }, }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: StringPtr("f2"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 254, + Line: 10, + Column: 15, + }, + Value: "f2", + }, }, }, }, @@ -958,7 +1069,14 @@ var validUnpackTestCases = []struct { inner: &configurableInner[string]{ single: singleConfigurable[string]{ cases: []ConfigurableCase[string]{{ - value: StringPtr("asdf"), + value: &parser.String{ + LiteralPos: scanner.Position{ + Offset: 25, + Line: 2, + Column: 25, + }, + Value: "asdf", + }, }}, }, }, @@ -968,7 +1086,15 @@ var validUnpackTestCases = []struct { inner: &configurableInner[bool]{ single: singleConfigurable[bool]{ cases: []ConfigurableCase[bool]{{ - value: BoolPtr(true), + value: &parser.Bool{ + LiteralPos: scanner.Position{ + Offset: 54, + Line: 3, + Column: 23, + }, + Value: true, + Token: "true", + }, }}, }, },