diff options
author | Cole Faust <colefaust@google.com> | 2024-04-10 14:57:34 -0700 |
---|---|---|
committer | Cole Faust <colefaust@google.com> | 2024-04-12 16:33:01 -0700 |
commit | 3311debbb3f37e0dc5246ad22b9a2d9cccc3f27a (patch) | |
tree | 8e81058a91c96557f27a8f66b0cde1eed7604427 | |
parent | 09fe90e407fcc2ac93b1641792cff5c95d2f8318 (diff) | |
download | blueprint-3311debbb3f37e0dc5246ad22b9a2d9cccc3f27a.tar.gz |
Support multi-variable selects and typed selects
This adds support for selecting on multiple variables at once, so that
you can do AND/OR combindations of them. For example:
select((
arch(),
os(),
), {
("arm64", "linux"): ["libfoo64"],
(default, "linux"): ["libfoo"],
(default, "windows"): ["libfoowindows"],
(default, default): ["libbar"],
})
It also allows for select conditions to be boolean-typed. You can
write literal true and false without quotes to select on them. Currently
we don't have any boolean-typed variables though, so a fake one was
added for testing.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Ibe586e7b21865b8734027848cc421594cbd1d8cc
-rw-r--r-- | parser/ast.go | 64 | ||||
-rw-r--r-- | parser/parser.go | 252 | ||||
-rw-r--r-- | parser/parser_test.go | 11 | ||||
-rw-r--r-- | parser/printer.go | 101 | ||||
-rw-r--r-- | parser/printer_test.go | 46 | ||||
-rw-r--r-- | proptools/configurable.go | 294 | ||||
-rw-r--r-- | proptools/extend_test.go | 150 | ||||
-rw-r--r-- | proptools/unpack.go | 117 | ||||
-rw-r--r-- | proptools/unpack_test.go | 123 |
9 files changed, 842 insertions, 316 deletions
diff --git a/parser/ast.go b/parser/ast.go index 68f15ed..7aea5e0 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -571,37 +571,46 @@ func endPos(pos scanner.Position, n int) scanner.Position { return pos } -type SelectType int - -const ( - SelectTypeUnconfigured SelectType = iota // Used for selects with only one branch, which is "default" - SelectTypeReleaseVariable - SelectTypeSoongConfigVariable - SelectTypeProductVariable - SelectTypeVariant -) +type ConfigurableCondition struct { + position scanner.Position + FunctionName string + Args []String +} -func (s SelectType) String() string { - switch s { - case SelectTypeUnconfigured: - return "unconfigured" - case SelectTypeReleaseVariable: - return "release variable" - case SelectTypeSoongConfigVariable: - return "soong config variable" - case SelectTypeProductVariable: - return "product variable" - case SelectTypeVariant: - return "variant" - default: - panic("unreachable") +func (c *ConfigurableCondition) Equals(other ConfigurableCondition) bool { + if c.FunctionName != other.FunctionName { + return false + } + if len(c.Args) != len(other.Args) { + return false + } + for i := range c.Args { + if c.Args[i] != other.Args[i] { + return false + } + } + return true +} + +func (c *ConfigurableCondition) String() string { + var sb strings.Builder + sb.WriteString(c.FunctionName) + sb.WriteRune('(') + for i, arg := range c.Args { + sb.WriteRune('"') + sb.WriteString(arg.Value) + sb.WriteRune('"') + if i < len(c.Args)-1 { + sb.WriteString(", ") + } } + sb.WriteRune(')') + return sb.String() } type Select struct { KeywordPos scanner.Position // the keyword "select" - Typ SelectType - Condition String + Conditions []ConfigurableCondition LBracePos scanner.Position RBracePos scanner.Position Cases []*SelectCase // the case statements @@ -640,8 +649,7 @@ func (s *Select) Type() Type { } type SelectCase struct { - // TODO: Support int and bool typed cases - Pattern String + Patterns []Expression ColonPos scanner.Position Value Expression } @@ -656,7 +664,7 @@ func (c *SelectCase) String() string { return "<select case>" } -func (c *SelectCase) Pos() scanner.Position { return c.Pattern.LiteralPos } +func (c *SelectCase) Pos() scanner.Position { return c.Patterns[0].Pos() } func (c *SelectCase) End() scanner.Position { return c.Value.End() } // UnsetProperty is the expression type of the "unset" keyword that can be diff --git a/parser/parser.go b/parser/parser.go index 07f4681..eaed054 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -28,6 +28,8 @@ var errTooManyErrors = errors.New("too many errors") const maxErrors = 1 +const default_select_branch_name = "__soong_conditions_default__" + type ParseError struct { Err error Pos scanner.Position @@ -177,7 +179,6 @@ func (p *parser) next() { p.comments = append(p.comments, &CommentGroup{Comments: comments}) } } - return } func (p *parser) parseDefinitions() (defs []Definition) { @@ -386,11 +387,7 @@ func (p *parser) evaluateOperator(value1, value2 Expression, operator rune, if _, ok := e2.(*Select); ok { // Promote e1 to a select so we can add e2 to it e1 = &Select{ - Typ: SelectTypeUnconfigured, Cases: []*SelectCase{{ - Pattern: String{ - Value: "__soong_conditions_default__", - }, Value: e1, }}, } @@ -563,49 +560,81 @@ func (p *parser) parseSelect() Expression { result := &Select{ KeywordPos: p.scanner.Position, } - p.accept(scanner.Ident) - if !p.accept('(') { - return nil - } - switch p.scanner.TokenText() { - case "release_variable": - result.Typ = SelectTypeReleaseVariable - case "soong_config_variable": - result.Typ = SelectTypeSoongConfigVariable - case "product_variable": - result.Typ = SelectTypeProductVariable - case "variant": - result.Typ = SelectTypeVariant - default: - p.errorf("unknown select type %q, expected release_variable, soong_config_variable, product_variable, or variant", p.scanner.TokenText()) - return nil - } + // Read the "select(" p.accept(scanner.Ident) if !p.accept('(') { return nil } - if s := p.parseStringValue(); s != nil { - result.Condition = *s - } else { - return nil + // If we see another '(', there's probably multiple conditions and there must + // be a ')' after. Set the multipleConditions variable to remind us to check for + // the ')' after. + multipleConditions := false + if p.tok == '(' { + multipleConditions = true + p.accept('(') } - if result.Typ == SelectTypeSoongConfigVariable { - if !p.accept(',') { + // Read all individual conditions + conditions := []ConfigurableCondition{} + for first := true; first || multipleConditions; first = false { + condition := ConfigurableCondition{ + position: p.scanner.Position, + FunctionName: p.scanner.TokenText(), + } + if !p.accept(scanner.Ident) { return nil } - if s := p.parseStringValue(); s != nil { - result.Condition.Value += ":" + s.Value - } else { + if !p.accept('(') { return nil } + + for p.tok != ')' { + if s := p.parseStringValue(); s != nil { + condition.Args = append(condition.Args, *s) + } else { + return nil + } + if p.tok == ')' { + break + } + if !p.accept(',') { + return nil + } + } + p.accept(')') + + for _, c := range conditions { + if c.Equals(condition) { + p.errorf("Duplicate select condition found: %s", c.String()) + } + } + + conditions = append(conditions, condition) + + if multipleConditions { + if p.tok == ')' { + p.next() + break + } + if !p.accept(',') { + return nil + } + // Retry the closing parent to allow for a trailing comma + if p.tok == ')' { + p.next() + break + } + } } - if !p.accept(')') { + if multipleConditions && len(conditions) < 2 { + p.errorf("Expected multiple select conditions due to the extra parenthesis, but only found 1. Please remove the extra parenthesis.") return nil } + result.Conditions = conditions + if !p.accept(',') { return nil } @@ -615,17 +644,78 @@ func (p *parser) parseSelect() Expression { return nil } + parseOnePattern := func() Expression { + switch p.tok { + case scanner.Ident: + switch p.scanner.TokenText() { + case "default": + p.next() + return &String{ + LiteralPos: p.scanner.Position, + Value: default_select_branch_name, + } + case "true": + p.next() + return &Bool{ + LiteralPos: p.scanner.Position, + Value: true, + } + case "false": + p.next() + return &Bool{ + LiteralPos: p.scanner.Position, + Value: false, + } + default: + p.errorf("Expted a string, true, false, or default, got %s", p.scanner.TokenText()) + } + case scanner.String: + if s := p.parseStringValue(); s != nil { + if strings.HasPrefix(s.Value, "__soong") { + p.errorf("select branch conditions starting with __soong are reserved for internal use") + return nil + } + return s + } + fallthrough + default: + p.errorf("Expted a string, true, false, or default, got %s", p.scanner.TokenText()) + } + return nil + } + hasNonUnsetValue := false - for p.tok == scanner.String { + for p.tok != '}' { c := &SelectCase{} - if s := p.parseStringValue(); s != nil { - if strings.HasPrefix(s.Value, "__soong") { - p.errorf("select branch conditions starting with __soong are reserved for internal use") + + if multipleConditions { + if !p.accept('(') { + return nil + } + for i := 0; i < len(conditions); i++ { + if p := parseOnePattern(); p != nil { + c.Patterns = append(c.Patterns, p) + } else { + return nil + } + if i < len(conditions)-1 { + if !p.accept(',') { + return nil + } + } else if p.tok == ',' { + // allow optional trailing comma + p.next() + } + } + if !p.accept(')') { return nil } - c.Pattern = *s } else { - return nil + if p := parseOnePattern(); p != nil { + c.Patterns = append(c.Patterns, p) + } else { + return nil + } } c.ColonPos = p.scanner.Position if !p.accept(':') { @@ -641,44 +731,70 @@ func (p *parser) parseSelect() Expression { if !p.accept(',') { return nil } - result.Cases = append(result.Cases, c) } - // Default must be last - if p.tok == scanner.Ident { - if p.scanner.TokenText() != "default" { - p.errorf("select cases can either be quoted strings or 'default' to match any value") - return nil - } - c := &SelectCase{Pattern: String{ - LiteralPos: p.scanner.Position, - Value: "__soong_conditions_default__", - }} - p.accept(scanner.Ident) - c.ColonPos = p.scanner.Position - if !p.accept(':') { - return nil + // If all branches have the value "unset", then this is equivalent + // to an empty select. + if !hasNonUnsetValue { + p.errorf("This select statement is empty, remove it") + return nil + } + + patternsEqual := func(a, b Expression) bool { + switch a2 := a.(type) { + case *String: + if b2, ok := b.(*String); ok { + return a2.Value == b2.Value + } else { + return false + } + case *Bool: + if b2, ok := b.(*Bool); ok { + return a2.Value == b2.Value + } else { + return false + } + default: + // true so that we produce an error in this unexpected scenario + return true } - if p.tok == scanner.Ident && p.scanner.TokenText() == "unset" { - c.Value = UnsetProperty{Position: p.scanner.Position} - p.accept(scanner.Ident) - } else { - hasNonUnsetValue = true - c.Value = p.parseExpression() + } + + patternListsEqual := func(a, b []Expression) bool { + if len(a) != len(b) { + return false } - if !p.accept(',') { - return nil + for i := range a { + if !patternsEqual(a[i], b[i]) { + return false + } } - result.Cases = append(result.Cases, c) + return true } - // If all branches have the value "unset", then this is equivalent - // to an empty select. - if !hasNonUnsetValue { - result.Typ = SelectTypeUnconfigured - result.Condition.Value = "" - result.Cases = nil + for i, c := range result.Cases { + // Check for duplicates + for _, d := range result.Cases[i+1:] { + if patternListsEqual(c.Patterns, d.Patterns) { + p.errorf("Found duplicate select patterns: %v", c.Patterns) + return nil + } + } + // Check that the only all-default cases is the last one + if i < len(result.Cases)-1 { + isAllDefault := true + for _, x := range c.Patterns { + if x2, ok := x.(*String); !ok || x2.Value != default_select_branch_name { + isAllDefault = false + break + } + } + if isAllDefault { + p.errorf("Found a default select branch at index %d, expected it to be last (index %d)", i, len(result.Cases)-1) + return nil + } + } } ty := UnsetType diff --git a/parser/parser_test.go b/parser/parser_test.go index b393792..9de69c0 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -1225,6 +1225,17 @@ func TestParserError(t *testing.T) { input: "\x00", err: "invalid character NUL", }, + { + name: "select with duplicate condition", + input: ` + m { + foo: select((arch(), arch()), { + (default, default): true, + }), + } + `, + err: "Duplicate select condition found: arch()", + }, // TODO: test more parser errors } diff --git a/parser/printer.go b/parser/printer.go index 9d83e7a..848b95a 100644 --- a/parser/printer.go +++ b/parser/printer.go @@ -142,48 +142,83 @@ func (p *printer) printSelect(s *Select) { if len(s.Cases) == 0 { return } - if len(s.Cases) == 1 && s.Cases[0].Pattern.Value == "__soong_conditions_default__" { - p.printExpression(s.Cases[0].Value) - p.pos = s.RBracePos - return + if len(s.Cases) == 1 && len(s.Cases[0].Patterns) == 1 { + if str, ok := s.Cases[0].Patterns[0].(*String); ok && str.Value == default_select_branch_name { + p.printExpression(s.Cases[0].Value) + p.pos = s.RBracePos + return + } } p.requestSpace() p.printToken("select(", s.KeywordPos) - switch s.Typ { - case SelectTypeSoongConfigVariable: - p.printToken("soong_config_variable(", s.Condition.LiteralPos) - parts := strings.Split(s.Condition.Value, ":") - namespace := parts[0] - variable := parts[1] - p.printToken(strconv.Quote(namespace), s.Condition.LiteralPos) - p.printToken(",", s.Condition.LiteralPos) - p.requestSpace() - p.printToken(strconv.Quote(variable), s.Condition.LiteralPos) - p.printToken(")", s.Condition.LiteralPos) - case SelectTypeReleaseVariable: - p.printToken("release_variable(", s.Condition.LiteralPos) - p.printToken(strconv.Quote(s.Condition.Value), s.Condition.LiteralPos) - p.printToken(")", s.Condition.LiteralPos) - case SelectTypeProductVariable: - p.printToken("product_variable(", s.Condition.LiteralPos) - p.printToken(strconv.Quote(s.Condition.Value), s.Condition.LiteralPos) - p.printToken(")", s.Condition.LiteralPos) - case SelectTypeVariant: - p.printToken("variant(", s.Condition.LiteralPos) - p.printToken(strconv.Quote(s.Condition.Value), s.Condition.LiteralPos) - p.printToken(")", s.Condition.LiteralPos) - default: - panic("should be unreachable") + multilineConditions := false + if len(s.Conditions) > 1 { + p.printToken("(", s.KeywordPos) + if s.Conditions[len(s.Conditions)-1].position.Line > s.KeywordPos.Line { + multilineConditions = true + p.requestNewline() + p.indent(p.curIndent() + 4) + } + } + for i, c := range s.Conditions { + p.printToken(c.FunctionName, c.position) + p.printToken("(", c.position) + for i, arg := range c.Args { + p.printToken(strconv.Quote(arg.Value), arg.LiteralPos) + if i < len(c.Args)-1 { + p.printToken(",", arg.LiteralPos) + p.requestSpace() + } + } + p.printToken(")", p.pos) + if len(s.Conditions) > 1 { + if multilineConditions { + p.printToken(",", p.pos) + p.requestNewline() + } else if i < len(s.Conditions)-1 { + p.printToken(",", p.pos) + p.requestSpace() + } + } + } + if len(s.Conditions) > 1 { + if multilineConditions { + p.unindent(p.pos) + } + p.printToken(")", p.pos) } p.printToken(", {", s.LBracePos) p.requestNewline() p.indent(p.curIndent() + 4) for _, c := range s.Cases { p.requestNewline() - if c.Pattern.Value != "__soong_conditions_default__" { - p.printToken(strconv.Quote(c.Pattern.Value), c.Pattern.LiteralPos) - } else { - p.printToken("default", c.Pattern.LiteralPos) + if len(c.Patterns) > 1 { + p.printToken("(", p.pos) + } + for i, pat := range c.Patterns { + switch pat := pat.(type) { + case *String: + if pat.Value != default_select_branch_name { + p.printToken(strconv.Quote(pat.Value), pat.LiteralPos) + } else { + p.printToken("default", pat.LiteralPos) + } + case *Bool: + s := "false" + if pat.Value { + s = "true" + } + p.printToken(s, pat.LiteralPos) + default: + panic("Unhandled case") + } + if i < len(c.Patterns)-1 { + p.printToken(",", p.pos) + p.requestSpace() + } + } + if len(c.Patterns) > 1 { + p.printToken(")", p.pos) } p.printToken(":", c.ColonPos) p.requestSpace() diff --git a/parser/printer_test.go b/parser/printer_test.go index e273115..3a81551 100644 --- a/parser/printer_test.go +++ b/parser/printer_test.go @@ -657,50 +657,44 @@ foo { `, }, { - name: "Select with only unsets is removed", + name: "Multi-condition select", input: ` foo { - stuff: select(soong_config_variable("my_namespace", "my_variable"), { - "foo": unset, - default: unset, + stuff: select((arch(), os()), { + ("x86", "linux"): "a", + (default, default): "b", }), } `, output: ` foo { - + stuff: select((arch(), os()), { + ("x86", "linux"): "a", + (default, default): "b", + }), } `, }, { - name: "Additions of unset selects are removed", + name: "Multi-condition select with conditions on new lines", input: ` foo { - stuff: select(soong_config_variable("my_namespace", "my_variable"), { - "foo": "a", - default: "b", - }) + select(soong_config_variable("my_namespace", "my_variable2"), { - "foo": unset, - default: unset, - }) + select(soong_config_variable("my_namespace", "my_variable3"), { - "foo": "c", - default: "d", + stuff: select((arch(), + os()), { + ("x86", "linux"): "a", + (default, default): "b", }), } `, - // TODO(b/323382414): This is not good formatting, revisit later. - // But at least it removes the useless middle select output: ` foo { - stuff: select(soong_config_variable("my_namespace", "my_variable"), { - "foo": "a", - default: "b", - }) + - - select(soong_config_variable("my_namespace", "my_variable3"), { - "foo": "c", - default: "d", - }), + stuff: select(( + arch(), + os(), + ), { + ("x86", "linux"): "a", + (default, default): "b", + }), } `, }, diff --git a/proptools/configurable.go b/proptools/configurable.go index 6d88fae..b136f59 100644 --- a/proptools/configurable.go +++ b/proptools/configurable.go @@ -14,21 +14,18 @@ package proptools import ( - "fmt" "reflect" "slices" - - "github.com/google/blueprint/parser" + "strconv" + "strings" ) -const default_select_branch_name = "__soong_conditions_default__" - type ConfigurableElements interface { string | bool | []string } type ConfigurableEvaluator interface { - EvaluateConfiguration(typ parser.SelectType, property, condition string) (string, bool) + EvaluateConfiguration(condition ConfigurableCondition, property string) ConfigurableValue PropertyErrorf(property, fmt string, args ...interface{}) } @@ -38,6 +35,200 @@ type configurableMarker bool var configurableMarkerType reflect.Type = reflect.TypeOf((*configurableMarker)(nil)).Elem() +type ConfigurableCondition struct { + FunctionName string + Args []string +} + +func (c *ConfigurableCondition) String() string { + var sb strings.Builder + sb.WriteString(c.FunctionName) + sb.WriteRune('(') + for i, arg := range c.Args { + sb.WriteString(strconv.Quote(arg)) + if i < len(c.Args)-1 { + sb.WriteString(", ") + } + } + sb.WriteRune(')') + return sb.String() +} + +type configurableValueType int + +const ( + configurableValueTypeString configurableValueType = iota + configurableValueTypeBool + configurableValueTypeUndefined +) + +func (v *configurableValueType) patternType() configurablePatternType { + switch *v { + case configurableValueTypeString: + return configurablePatternTypeString + case configurableValueTypeBool: + return configurablePatternTypeBool + default: + panic("unimplemented") + } +} + +func (v *configurableValueType) String() string { + switch *v { + case configurableValueTypeString: + return "string" + case configurableValueTypeBool: + return "bool" + case configurableValueTypeUndefined: + return "undefined" + default: + panic("unimplemented") + } +} + +// ConfigurableValue represents the value of a certain condition being selected on. +// This type mostly exists to act as a sum type between string, bool, and undefined. +type ConfigurableValue struct { + typ configurableValueType + stringValue string + boolValue bool +} + +func (c *ConfigurableValue) String() string { + switch c.typ { + case configurableValueTypeString: + return strconv.Quote(c.stringValue) + case configurableValueTypeBool: + if c.boolValue { + return "true" + } else { + return "false" + } + case configurableValueTypeUndefined: + return "undefined" + default: + panic("unimplemented") + } +} + +func ConfigurableValueString(s string) ConfigurableValue { + return ConfigurableValue{ + typ: configurableValueTypeString, + stringValue: s, + } +} + +func ConfigurableValueBool(b bool) ConfigurableValue { + return ConfigurableValue{ + typ: configurableValueTypeBool, + boolValue: b, + } +} + +func ConfigurableValueUndefined() ConfigurableValue { + return ConfigurableValue{ + typ: configurableValueTypeUndefined, + } +} + +type configurablePatternType int + +const ( + configurablePatternTypeString configurablePatternType = iota + configurablePatternTypeBool + configurablePatternTypeDefault +) + +func (v *configurablePatternType) String() string { + switch *v { + case configurablePatternTypeString: + return "string" + case configurablePatternTypeBool: + return "bool" + case configurablePatternTypeDefault: + return "default" + default: + panic("unimplemented") + } +} + +type configurablePattern struct { + typ configurablePatternType + stringValue string + boolValue bool +} + +func (p *configurablePattern) matchesValue(v ConfigurableValue) bool { + if p.typ == configurablePatternTypeDefault { + return true + } + if v.typ == configurableValueTypeUndefined { + return false + } + if p.typ != v.typ.patternType() { + return false + } + switch p.typ { + case configurablePatternTypeString: + return p.stringValue == v.stringValue + case configurablePatternTypeBool: + return p.boolValue == v.boolValue + default: + panic("unimplemented") + } +} + +func (p *configurablePattern) matchesValueType(v ConfigurableValue) bool { + if p.typ == configurablePatternTypeDefault { + return true + } + if v.typ == configurableValueTypeUndefined { + return true + } + return p.typ == v.typ.patternType() +} + +type configurableCase[T ConfigurableElements] struct { + patterns []configurablePattern + value *T +} + +func (c *configurableCase[T]) Clone() configurableCase[T] { + return configurableCase[T]{ + patterns: slices.Clone(c.patterns), + value: copyConfiguredValue(c.value), + } +} + +type configurableCaseReflection interface { + initialize(patterns []configurablePattern, value interface{}) +} + +var _ configurableCaseReflection = &configurableCase[string]{} + +func (c *configurableCase[T]) initialize(patterns []configurablePattern, value interface{}) { + c.patterns = patterns + c.value = value.(*T) +} + +// for the given T, return the reflect.type of configurableCase[T] +func configurableCaseType(configuredType reflect.Type) reflect.Type { + // I don't think it's possible to do this generically with go's + // current reflection apis unfortunately + switch configuredType.Kind() { + case reflect.String: + return reflect.TypeOf(configurableCase[string]{}) + case reflect.Bool: + return reflect.TypeOf(configurableCase[bool]{}) + case reflect.Slice: + switch configuredType.Elem().Kind() { + case reflect.String: + return reflect.TypeOf(configurableCase[[]string]{}) + } + } + panic("unimplemented") +} + // Configurable can wrap the type of a blueprint property, // in order to allow select statements to be used in bp files // for that property. For example, for the property struct: @@ -51,11 +242,11 @@ var configurableMarkerType reflect.Type = reflect.TypeOf((*configurableMarker)(n // // my_module { // property_a: "foo" -// property_b: select soong_config_variable: "my_namespace" "my_variable" { +// property_b: select(soong_config_variable("my_namespace", "my_variable"), { // "value_1": "bar", // "value_2": "baz", // default: "qux", -// } +// }) // } // // The configurable property holds all the branches of the select @@ -67,9 +258,8 @@ var configurableMarkerType reflect.Type = reflect.TypeOf((*configurableMarker)(n type Configurable[T ConfigurableElements] struct { marker configurableMarker propertyName string - typ parser.SelectType - condition string - cases map[string]*T + conditions []ConfigurableCondition + cases []configurableCase[T] appendWrapper *appendWrapper[T] } @@ -112,40 +302,49 @@ func (c *Configurable[T]) GetOrDefault(evaluator ConfigurableEvaluator, defaultV } func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) *T { - if c.typ == parser.SelectTypeUnconfigured { - if len(c.cases) == 0 { + for i, case_ := range c.cases { + if len(c.conditions) != len(case_.patterns) { + evaluator.PropertyErrorf(c.propertyName, "Expected each case to have as many patterns as conditions. conditions: %d, len(cases[%d].patterns): %d", len(c.conditions), i, len(case_.patterns)) return nil - } else if len(c.cases) != 1 { - panic(fmt.Sprintf("Expected 0 or 1 branches in an unconfigured select, found %d", len(c.cases))) } - result, ok := c.cases[default_select_branch_name] - if !ok { - actual := "" - for k := range c.cases { - actual = k - } - panic(fmt.Sprintf("Expected the single branch of an unconfigured select to be %q, got %q", default_select_branch_name, actual)) - } - return result } - val, defined := evaluator.EvaluateConfiguration(c.typ, c.propertyName, c.condition) - if !defined { - if result, ok := c.cases[default_select_branch_name]; ok { - return result + if len(c.conditions) == 0 { + if len(c.cases) == 0 { + return nil + } else if len(c.cases) == 1 { + return c.cases[0].value + } else { + evaluator.PropertyErrorf(c.propertyName, "Expected 0 or 1 branches in an unconfigured select, found %d", len(c.cases)) + return nil } - evaluator.PropertyErrorf(c.propertyName, "%s %q was not defined", c.typ.String(), c.condition) - return nil } - if val == default_select_branch_name { - panic("Evaluator cannot return the default branch") + values := make([]ConfigurableValue, len(c.conditions)) + for i, condition := range c.conditions { + values[i] = evaluator.EvaluateConfiguration(condition, c.propertyName) } - if result, ok := c.cases[val]; ok { - return result + foundMatch := false + var result *T + for _, case_ := range c.cases { + allMatch := true + for i, pat := range case_.patterns { + if !pat.matchesValueType(values[i]) { + evaluator.PropertyErrorf(c.propertyName, "Expected all branches of a select on condition %s to have type %s, found %s", c.conditions[i].String(), values[i].typ.String(), pat.typ.String()) + return nil + } + if !pat.matchesValue(values[i]) { + allMatch = false + break + } + } + if allMatch && !foundMatch { + result = case_.value + foundMatch = true + } } - if result, ok := c.cases[default_select_branch_name]; ok { + if foundMatch { return result } - evaluator.PropertyErrorf(c.propertyName, "%s %q had value %q, which was not handled by the select statement", c.typ.String(), c.condition, val) + evaluator.PropertyErrorf(c.propertyName, "%s had value %s, which was not handled by the select statement", c.conditions, values) return nil } @@ -216,17 +415,16 @@ 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, typ parser.SelectType, condition string, cases any) + initialize(propertyName string, conditions []ConfigurableCondition, cases any) } var _ configurableReflection = Configurable[string]{} var _ configurablePtrReflection = &Configurable[string]{} -func (c *Configurable[T]) initialize(propertyName string, typ parser.SelectType, condition string, cases any) { +func (c *Configurable[T]) initialize(propertyName string, conditions []ConfigurableCondition, cases any) { c.propertyName = propertyName - c.typ = typ - c.condition = condition - c.cases = cases.(map[string]*T) + c.conditions = conditions + c.cases = cases.([]configurableCase[T]) c.appendWrapper = &appendWrapper[T]{} } @@ -243,7 +441,7 @@ func (c Configurable[T]) isEmpty() bool { if c.appendWrapper != nil && !c.appendWrapper.append.isEmpty() { return false } - return c.typ == parser.SelectTypeUnconfigured && len(c.cases) == 0 + return len(c.cases) == 0 } func (c Configurable[T]) configuredType() reflect.Type { @@ -267,15 +465,17 @@ func (c *Configurable[T]) clone() *Configurable[T] { } } - casesCopy := make(map[string]*T, len(c.cases)) - for k, v := range c.cases { - casesCopy[k] = copyConfiguredValue(v) + conditionsCopy := make([]ConfigurableCondition, len(c.conditions)) + copy(conditionsCopy, c.conditions) + + casesCopy := make([]configurableCase[T], len(c.cases)) + for i, case_ := range c.cases { + casesCopy[i] = case_.Clone() } return &Configurable[T]{ propertyName: c.propertyName, - typ: c.typ, - condition: c.condition, + conditions: conditionsCopy, cases: casesCopy, appendWrapper: inner, } diff --git a/proptools/extend_test.go b/proptools/extend_test.go index 498ce29..9b07c78 100644 --- a/proptools/extend_test.go +++ b/proptools/extend_test.go @@ -20,8 +20,6 @@ import ( "reflect" "strings" "testing" - - "github.com/google/blueprint/parser" ) type appendPropertyTestCase struct { @@ -1260,38 +1258,72 @@ func appendPropertiesTestCases() []appendPropertyTestCase { name: "Append configurable", dst: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeSoongConfigVariable, - condition: "foo", - cases: map[string]*[]string{ - "a": {"1", "2"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, src: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeReleaseVariable, - condition: "bar", - cases: map[string]*[]string{ - "b": {"3", "4"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "release_variable", + Args: []string{ + "bar", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, out: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeSoongConfigVariable, - condition: "foo", - cases: map[string]*[]string{ - "a": {"1", "2"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, appendWrapper: &appendWrapper[[]string]{ append: Configurable[[]string]{ - typ: parser.SelectTypeReleaseVariable, - condition: "bar", - cases: map[string]*[]string{ - "b": {"3", "4"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "release_variable", + Args: []string{ + "bar", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, @@ -1303,38 +1335,72 @@ func appendPropertiesTestCases() []appendPropertyTestCase { order: Prepend, dst: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeSoongConfigVariable, - condition: "foo", - cases: map[string]*[]string{ - "a": {"1", "2"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, src: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeReleaseVariable, - condition: "bar", - cases: map[string]*[]string{ - "b": {"3", "4"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "release_variable", + Args: []string{ + "bar", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, out: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - typ: parser.SelectTypeReleaseVariable, - condition: "bar", - cases: map[string]*[]string{ - "b": {"3", "4"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "release_variable", + Args: []string{ + "bar", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, appendWrapper: &appendWrapper[[]string]{ append: Configurable[[]string]{ - typ: parser.SelectTypeSoongConfigVariable, - condition: "foo", - cases: map[string]*[]string{ - "a": {"1", "2"}, - }, + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []configurableCase[[]string]{{ + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, diff --git a/proptools/unpack.go b/proptools/unpack.go index 85709cc..53b2b2f 100644 --- a/proptools/unpack.go +++ b/proptools/unpack.go @@ -356,10 +356,9 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } result := Configurable[string]{ propertyName: property.Name, - typ: parser.SelectTypeUnconfigured, - cases: map[string]*string{ - default_select_branch_name: &v.Value, - }, + cases: []configurableCase[string]{{ + value: &v.Value, + }}, appendWrapper: &appendWrapper[string]{}, } return reflect.ValueOf(&result), true @@ -374,10 +373,9 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } result := Configurable[bool]{ propertyName: property.Name, - typ: parser.SelectTypeUnconfigured, - cases: map[string]*bool{ - default_select_branch_name: &v.Value, - }, + cases: []configurableCase[bool]{{ + value: &v.Value, + }}, appendWrapper: &appendWrapper[bool]{}, } return reflect.ValueOf(&result), true @@ -392,9 +390,9 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } switch configuredType.Elem().Kind() { case reflect.String: - var cases map[string]*[]string + var value []string if v.Values != nil { - value := make([]string, 0, len(v.Values)) + value = make([]string, len(v.Values)) itemProperty := &parser.Property{NamePos: property.NamePos, ColonPos: property.ColonPos} for i, expr := range v.Values { itemProperty.Name = propertyName + "[" + strconv.Itoa(i) + "]" @@ -404,14 +402,14 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa ctx.addError(err) return reflect.ValueOf(Configurable[[]string]{}), false } - value = append(value, exprUnpacked.Interface().(string)) + value[i] = exprUnpacked.Interface().(string) } - cases = map[string]*[]string{default_select_branch_name: &value} } result := Configurable[[]string]{ - propertyName: property.Name, - typ: parser.SelectTypeUnconfigured, - cases: cases, + propertyName: property.Name, + cases: []configurableCase[[]string]{{ + value: &value, + }}, appendWrapper: &appendWrapper[[]string]{}, } return reflect.ValueOf(&result), true @@ -424,46 +422,81 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa case *parser.Select: resultPtr := reflect.New(configurableType) result := resultPtr.Elem() - cases := reflect.MakeMapWithSize(reflect.MapOf(reflect.TypeOf(""), reflect.PointerTo(configuredType)), len(v.Cases)) + conditions := make([]ConfigurableCondition, len(v.Conditions)) + for i, cond := range v.Conditions { + args := make([]string, len(cond.Args)) + for j, arg := range cond.Args { + args[j] = arg.Value + } + conditions[i] = ConfigurableCondition{ + FunctionName: cond.FunctionName, + Args: args, + } + } + + 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, } + + patterns := make([]configurablePattern, len(c.Patterns)) + for i, pat := range c.Patterns { + switch pat := pat.(type) { + case *parser.String: + if pat.Value == "__soong_conditions_default__" { + patterns[i].typ = configurablePatternTypeDefault + } else { + patterns[i].typ = configurablePatternTypeString + patterns[i].stringValue = pat.Value + } + case *parser.Bool: + patterns[i].typ = configurablePatternTypeBool + patterns[i].boolValue = pat.Value + default: + panic("unimplemented") + } + } + + var value reflect.Value // Map the "unset" keyword to a nil pointer in the cases map if _, ok := c.Value.(parser.UnsetProperty); ok { - cases.SetMapIndex(reflect.ValueOf(c.Pattern.Value), reflect.Zero(reflect.PointerTo(configuredType))) - continue - } - switch configuredType.Kind() { - case reflect.String, reflect.Bool: - val, err := propertyToValue(reflect.PointerTo(configuredType), p) - if err != nil { - ctx.addError(&UnpackError{ - err, - c.Value.Pos(), - }) - return reflect.New(configurableType), false - } - cases.SetMapIndex(reflect.ValueOf(c.Pattern.Value), val) - case reflect.Slice: - if configuredType.Elem().Kind() != reflect.String { - panic("This should be unreachable because ConfigurableElements only accepts slices of strings") - } - val, ok := ctx.unpackToSlice(p.Name, p, reflect.PointerTo(configuredType)) - if !ok { - return reflect.New(configurableType), false + 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") } - cases.SetMapIndex(reflect.ValueOf(c.Pattern.Value), val) - 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()) + cases = reflect.Append(cases, case_.Elem()) } resultPtr.Interface().(configurablePtrReflection).initialize( property.Name, - v.Typ, - v.Condition.Value, + conditions, cases.Interface(), ) if v.Append != nil { diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go index 7b7b46b..3182c8b 100644 --- a/proptools/unpack_test.go +++ b/proptools/unpack_test.go @@ -734,10 +734,9 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - typ: parser.SelectTypeUnconfigured, - cases: map[string]*string{ - default_select_branch_name: StringPtr("bar"), - }, + cases: []configurableCase[string]{{ + value: StringPtr("bar"), + }}, appendWrapper: &appendWrapper[string]{}, }, }, @@ -756,10 +755,9 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[bool]{ propertyName: "foo", - typ: parser.SelectTypeUnconfigured, - cases: map[string]*bool{ - default_select_branch_name: BoolPtr(true), - }, + cases: []configurableCase[bool]{{ + value: BoolPtr(true), + }}, appendWrapper: &appendWrapper[bool]{}, }, }, @@ -778,10 +776,9 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[[]string]{ propertyName: "foo", - typ: parser.SelectTypeUnconfigured, - cases: map[string]*[]string{ - default_select_branch_name: {"a", "b"}, - }, + cases: []configurableCase[[]string]{{ + value: &[]string{"a", "b"}, + }}, appendWrapper: &appendWrapper[[]string]{}, }, }, @@ -804,12 +801,34 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - typ: parser.SelectTypeSoongConfigVariable, - condition: "my_namespace:my_variable", - cases: map[string]*string{ - "a": StringPtr("a2"), - "b": StringPtr("b2"), - default_select_branch_name: StringPtr("c2"), + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "my_variable", + }, + }}, + cases: []configurableCase[string]{ + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: StringPtr("a2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: StringPtr("b2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: StringPtr("c2"), + }, }, appendWrapper: &appendWrapper[string]{}, }, @@ -837,22 +856,66 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - typ: parser.SelectTypeSoongConfigVariable, - condition: "my_namespace:my_variable", - cases: map[string]*string{ - "a": StringPtr("a2"), - "b": StringPtr("b2"), - default_select_branch_name: StringPtr("c2"), + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "my_variable", + }, + }}, + cases: []configurableCase[string]{ + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: StringPtr("a2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: StringPtr("b2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: StringPtr("c2"), + }, }, appendWrapper: &appendWrapper[string]{ append: Configurable[string]{ propertyName: "foo", - typ: parser.SelectTypeSoongConfigVariable, - condition: "my_namespace:my_2nd_variable", - cases: map[string]*string{ - "d": StringPtr("d2"), - "e": StringPtr("e2"), - default_select_branch_name: StringPtr("f2"), + conditions: []ConfigurableCondition{{ + FunctionName: "soong_config_variable", + Args: []string{ + "my_namespace", + "my_2nd_variable", + }, + }}, + cases: []configurableCase[string]{ + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "d", + }}, + value: StringPtr("d2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "e", + }}, + value: StringPtr("e2"), + }, + { + patterns: []configurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: StringPtr("f2"), + }, }, appendWrapper: &appendWrapper[string]{}, }, |