From 6437d4e737dd1c339ed64374ee8010da8febeb92 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 1 Feb 2024 17:44:27 -0800 Subject: [PATCH] Select statements Select statements are a new blueprint feature inspired by bazel's select statements. They are essentially alternative syntax for soong config variables that require less boilerplate. In addition, they support making decisions based on a module's variant, which will eliminate the need for manual property struct manipulation, such as the arch mutator's arch: and target: properties. In order to support decisions based on the variant, select statements cannot be evaluated as soon as they're parsed. Instead, they must be stored in the property struct unevaluated. This means that individual properties need to change their type from say, string, to Configurable[string]. Currently, only configurable strings, bools, and string slices are supported, but more types can be added later. The module implementation must call my_property.Evaluate(ctx) in order to get the final, resolved value of the select statement. Bug: 323382414 Test: go tests Change-Id: I62f8721d7f0ac3d1df4a06d7eaa260a5aa7fcba3 --- Android.bp | 1 + parser/ast.go | 93 ++++++++++++ parser/parser.go | 183 ++++++++++++++++++++--- parser/printer.go | 64 ++++++++ parser/printer_test.go | 131 ++++++++++++++--- proptools/clone.go | 6 +- proptools/configurable.go | 281 ++++++++++++++++++++++++++++++++++++ proptools/extend.go | 46 +++++- proptools/extend_test.go | 87 +++++++++++ proptools/proptools.go | 8 + proptools/proptools_test.go | 40 +++++ proptools/tag.go | 2 +- proptools/unpack.go | 190 +++++++++++++++++++++++- proptools/unpack_test.go | 140 ++++++++++++++++++ 14 files changed, 1212 insertions(+), 60 deletions(-) create mode 100644 proptools/configurable.go diff --git a/Android.bp b/Android.bp index d0a16ad..60a5c42 100644 --- a/Android.bp +++ b/Android.bp @@ -117,6 +117,7 @@ bootstrap_go_package { ], srcs: [ "proptools/clone.go", + "proptools/configurable.go", "proptools/escape.go", "proptools/extend.go", "proptools/filter.go", diff --git a/parser/ast.go b/parser/ast.go index ea774e6..cf8bb29 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -567,3 +567,96 @@ func endPos(pos scanner.Position, n int) scanner.Position { pos.Column += n return pos } + +type SelectType int + +const ( + SelectTypeUnconfigured SelectType = iota // Used for selects with only one branch, which is "default" + SelectTypeReleaseVariable + SelectTypeSoongConfigVariable + SelectTypeProductVariable + SelectTypeVariant +) + +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") + } +} + +type Select struct { + KeywordPos scanner.Position // the keyword "select" + Typ SelectType + Condition String + LBracePos scanner.Position + RBracePos scanner.Position + Cases []*SelectCase // the case statements + Append Expression +} + +func (s *Select) Pos() scanner.Position { return s.KeywordPos } +func (s *Select) End() scanner.Position { return endPos(s.RBracePos, 1) } + +func (s *Select) Copy() Expression { + ret := *s + ret.Cases = make([]*SelectCase, len(ret.Cases)) + for i, selectCase := range s.Cases { + ret.Cases[i] = selectCase.Copy() + } + if s.Append != nil { + ret.Append = s.Append.Copy() + } + return &ret +} + +func (s *Select) Eval() Expression { + return s +} + +func (s *Select) String() string { + return "" +} + +func (c *SelectCase) Pos() scanner.Position { return c.Pattern.LiteralPos } +func (c *SelectCase) End() scanner.Position { return c.Value.End() } diff --git a/parser/parser.go b/parser/parser.go index 63a6ac1..31096f2 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -362,6 +362,21 @@ func (p *parser) evaluateOperator(value1, value2 Expression, operator rune, 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{ + Typ: SelectTypeUnconfigured, + Cases: []*SelectCase{{ + Pattern: String{ + Value: "__soong_conditions_default__", + }, + Value: e1, + }}, + } + } + } + value = e1.Copy() switch operator { @@ -380,6 +395,8 @@ func (p *parser) evaluateOperator(value1, value2 Expression, operator rune, 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()) } @@ -457,7 +474,14 @@ func (p *parser) parseOperator(value1 Expression) *Operator { func (p *parser) parseValue() (value Expression) { switch p.tok { case scanner.Ident: - return p.parseVariable() + switch text := p.scanner.TokenText(); text { + case "true", "false": + return p.parseBoolean() + case "select": + return p.parseSelect() + default: + return p.parseVariable() + } case '-', scanner.Int: // Integer might have '-' sign ahead ('+' is only treated as operator now) return p.parseIntValue() case scanner.String, scanner.RawString: @@ -473,40 +497,159 @@ func (p *parser) parseValue() (value Expression) { } } -func (p *parser) parseVariable() Expression { - var value Expression - +func (p *parser) parseBoolean() Expression { switch text := p.scanner.TokenText(); text { case "true", "false": - value = &Bool{ + result := &Bool{ LiteralPos: p.scanner.Position, Value: text == "true", Token: text, } + p.accept(scanner.Ident) + return result default: - 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 - } + p.errorf("Expected true/false, got %q", text) + return nil + } +} + +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 { - value = &NotEvaluated{} - } - value = &Variable{ - Name: text, - NamePos: p.scanner.Position, - Value: value, + 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) return value } +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 + } + p.accept(scanner.Ident) + if !p.accept('(') { + return nil + } + + if s := p.parseStringValue(); s != nil { + result.Condition = *s + } else { + return nil + } + + if result.Typ == SelectTypeSoongConfigVariable { + if !p.accept(',') { + return nil + } + if s := p.parseStringValue(); s != nil { + result.Condition.Value += ":" + s.Value + } else { + return nil + } + } + + if !p.accept(')') { + return nil + } + + if !p.accept(',') { + return nil + } + + result.LBracePos = p.scanner.Position + if !p.accept('{') { + return nil + } + + for p.tok == scanner.String { + 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") + return nil + } + c.Pattern = *s + } else { + return nil + } + c.ColonPos = p.scanner.Position + if !p.accept(':') { + return nil + } + c.Value = p.parseExpression() + if !p.accept(',') { + return nil + } + + result.Cases = append(result.Cases, c) + } + + // Default must be last + if p.tok == scanner.Ident { + if p.scanner.TokenText() != "_" { + p.errorf("select cases can either be quoted strings or '_' 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 + } + c.Value = p.parseExpression() + if !p.accept(',') { + return nil + } + result.Cases = append(result.Cases, c) + } + + result.RBracePos = p.scanner.Position + if !p.accept('}') { + return nil + } + if !p.accept(')') { + return nil + } + return result +} + func (p *parser) parseStringValue() *String { str, err := strconv.Unquote(p.scanner.TokenText()) if err != nil { diff --git a/parser/printer.go b/parser/printer.go index f377505..64063c8 100644 --- a/parser/printer.go +++ b/parser/printer.go @@ -131,11 +131,75 @@ func (p *printer) printExpression(value Expression) { p.printList(v.Values, v.LBracePos, v.RBracePos) case *Map: p.printMap(v) + case *Select: + p.printSelect(v) default: panic(fmt.Errorf("bad property type: %s", value.Type())) } } +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) + 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") + } + 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("_", c.Pattern.LiteralPos) + } + p.printToken(":", c.ColonPos) + p.requestSpace() + p.printExpression(c.Value) + p.printToken(",", c.Value.Pos()) + } + p.requestNewline() + p.unindent(s.RBracePos) + p.printToken("})", s.RBracePos) + if s.Append != nil { + p.requestSpace() + p.printToken("+", s.RBracePos) + p.requestSpace() + p.printExpression(s.Append) + } +} + func (p *printer) printList(list []Expression, pos, endPos scanner.Position) { p.requestSpace() p.printToken("[", pos) diff --git a/parser/printer_test.go b/parser/printer_test.go index b708441..30425c2 100644 --- a/parser/printer_test.go +++ b/parser/printer_test.go @@ -20,6 +20,7 @@ import ( ) var validPrinterTestCases = []struct { + name string input string output string }{ @@ -557,39 +558,123 @@ foo { "a", ], } +`, + }, + { + name: "Basic selects", + input: ` +// test +foo { + stuff: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + // test2 + "b": "b2", + // test3 + _: "c2", + }), +} +`, + output: ` +// test +foo { + stuff: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + // test2 + "b": "b2", + // test3 + _: "c2", + }), +} +`, + }, + { + name: "Remove select with only default", + input: ` +// test +foo { + stuff: select(soong_config_variable("my_namespace", "my_variable"), { + // test2 + _: "c2", + }), +} +`, + // TODO(b/323382414): This shouldn't have an empty newline after stuff + output: ` +// test +foo { + stuff: "c2", // test2 + +} +`, + }, + { + name: "Appended selects", + input: ` +// test +foo { + stuff: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + // test2 + "b": "b2", + // test3 + _: "c2", + }) + select(release_variable("RELEASE_TEST"), { + "d": "d2", + "e": "e2", + _: "f2", + }), +} +`, + output: ` +// test +foo { + stuff: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + // test2 + "b": "b2", + // test3 + _: "c2", + }) + select(release_variable("RELEASE_TEST"), { + "d": "d2", + "e": "e2", + _: "f2", + }), +} `, }, } func TestPrinter(t *testing.T) { for _, testCase := range validPrinterTestCases { - in := testCase.input[1:] - expected := testCase.output[1:] + t.Run(testCase.name, func(t *testing.T) { + in := testCase.input[1:] + expected := testCase.output[1:] - r := bytes.NewBufferString(in) - file, errs := Parse("", r, NewScope(nil)) - if len(errs) != 0 { - t.Errorf("test case: %s", in) - t.Errorf("unexpected errors:") - for _, err := range errs { - t.Errorf(" %s", err) + r := bytes.NewBufferString(in) + file, errs := Parse("", r, NewScope(nil)) + if len(errs) != 0 { + t.Errorf("test case: %s", in) + t.Errorf("unexpected errors:") + for _, err := range errs { + t.Errorf(" %s", err) + } + t.FailNow() } - t.FailNow() - } - SortLists(file) + SortLists(file) - got, err := Print(file) - if err != nil { - t.Errorf("test case: %s", in) - t.Errorf("unexpected error: %s", err) - t.FailNow() - } + got, err := Print(file) + if err != nil { + t.Errorf("test case: %s", in) + t.Errorf("unexpected error: %s", err) + t.FailNow() + } - if string(got) != expected { - t.Errorf("test case: %s", in) - t.Errorf(" expected: %s", expected) - t.Errorf(" got: %s", string(got)) - } + if string(got) != expected { + t.Errorf("test case: %s", in) + t.Errorf(" expected: %s", expected) + t.Errorf(" got: %s", string(got)) + } + }) } } diff --git a/proptools/clone.go b/proptools/clone.go index f464fa6..8ac5c6c 100644 --- a/proptools/clone.go +++ b/proptools/clone.go @@ -66,7 +66,11 @@ func copyProperties(dstValue, srcValue reflect.Value) { case reflect.Bool, reflect.String, reflect.Int, reflect.Uint: dstFieldValue.Set(srcFieldValue) case reflect.Struct: - copyProperties(dstFieldValue, srcFieldValue) + if isConfigurable(srcFieldValue.Type()) { + dstFieldValue.Set(srcFieldValue.Interface().(configurableReflection).cloneToReflectValuePtr().Elem()) + } else { + copyProperties(dstFieldValue, srcFieldValue) + } case reflect.Slice: if !srcFieldValue.IsNil() { if srcFieldValue != dstFieldValue { diff --git a/proptools/configurable.go b/proptools/configurable.go new file mode 100644 index 0000000..4ca92fc --- /dev/null +++ b/proptools/configurable.go @@ -0,0 +1,281 @@ +// Copyright 2023 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package proptools + +import ( + "fmt" + "reflect" + "slices" + + "github.com/google/blueprint/parser" +) + +const default_select_branch_name = "__soong_conditions_default__" + +type ConfigurableElements interface { + string | bool | []string +} + +type ConfigurableEvaluator interface { + EvaluateConfiguration(parser.SelectType, string) (string, bool) + PropertyErrorf(property, fmt string, args ...interface{}) +} + +// configurableMarker is just so that reflection can check type of the first field of +// the struct to determine if it is a configurable struct. +type configurableMarker bool + +var configurableMarkerType reflect.Type = reflect.TypeOf((*configurableMarker)(nil)).Elem() + +// 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: +// +// my_props { +// Property_a: string, +// Property_b: Configurable[string], +// } +// +// property_b can then use select statements: +// +// my_module { +// property_a: "foo" +// 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 +// statement in the bp file. To extract the final value, you must +// call Evaluate() on the configurable property. +// +// All configurable properties support being unset, so there is +// no need to use a pointer type like Configurable[*string]. +type Configurable[T ConfigurableElements] struct { + marker configurableMarker + propertyName string + typ parser.SelectType + condition string + cases map[string]T + appendWrapper *appendWrapper[T] +} + +// Ignore the warning about the unused marker variable, it's used via reflection +var _ configurableMarker = Configurable[string]{}.marker + +// appendWrapper exists so that we can set the value of append +// from a non-pointer method receiver. (setAppend) +type appendWrapper[T ConfigurableElements] struct { + append Configurable[T] +} + +func (c *Configurable[T]) GetType() parser.SelectType { + return c.typ +} + +func (c *Configurable[T]) GetCondition() string { + return c.condition +} + +// Evaluate returns the final value for the configurable property. +// A configurable property may be unset, in which case Evaluate will return nil. +func (c *Configurable[T]) Evaluate(evaluator ConfigurableEvaluator) *T { + if c == nil || c.appendWrapper == nil { + return nil + } + return mergeConfiguredValues( + c.evaluateNonTransitive(evaluator), + c.appendWrapper.append.Evaluate(evaluator), + c.propertyName, + evaluator, + ) +} + +func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) *T { + if c.typ == parser.SelectTypeUnconfigured { + if len(c.cases) == 0 { + 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.condition) + if !defined { + if result, ok := c.cases[default_select_branch_name]; ok { + return &result + } + 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") + } + if result, ok := c.cases[val]; ok { + return &result + } + if result, ok := c.cases[default_select_branch_name]; ok { + 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) + return nil +} + +func mergeConfiguredValues[T ConfigurableElements](a, b *T, propertyName string, evalutor ConfigurableEvaluator) *T { + if a == nil && b == nil { + return nil + } + switch any(a).(type) { + case *[]string: + var a2 []string + var b2 []string + if a != nil { + a2 = *any(a).(*[]string) + } + if b != nil { + b2 = *any(b).(*[]string) + } + result := make([]string, len(a2)+len(b2)) + idx := 0 + for i := 0; i < len(a2); i++ { + result[idx] = a2[i] + idx += 1 + } + for i := 0; i < len(b2); i++ { + result[idx] = b2[i] + idx += 1 + } + return any(&result).(*T) + case *string: + a := String(any(a).(*string)) + b := String(any(b).(*string)) + result := a + b + return any(&result).(*T) + case *bool: + numNonNil := 0 + var nonNil *T + if a != nil { + numNonNil += 1 + nonNil = a + } + if b != nil { + numNonNil += 1 + nonNil = b + } + if numNonNil == 1 { + return nonNil + } else { + evalutor.PropertyErrorf(propertyName, "Cannot append bools") + return nil + } + default: + panic("Should be unreachable") + } +} + +// configurableReflection is an interface that exposes some methods that are +// helpful when working with reflect.Values of Configurable objects, used by +// the property unpacking code. You can't call unexported methods from reflection, +// (at least without unsafe pointer trickery) so this is the next best thing. +type configurableReflection interface { + setAppend(append any) + configuredType() reflect.Type + cloneToReflectValuePtr() reflect.Value + isEmpty() bool +} + +// 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) +} + +var _ configurableReflection = Configurable[string]{} +var _ configurablePtrReflection = &Configurable[string]{} + +func (c *Configurable[T]) initialize(propertyName string, typ parser.SelectType, condition string, cases any) { + c.propertyName = propertyName + c.typ = typ + c.condition = condition + c.cases = cases.(map[string]T) + c.appendWrapper = &appendWrapper[T]{} +} + +func (c Configurable[T]) setAppend(append any) { + if c.appendWrapper.append.isEmpty() { + c.appendWrapper.append = append.(Configurable[T]) + } else { + c.appendWrapper.append.setAppend(append) + } +} + +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 +} + +func (c Configurable[T]) configuredType() reflect.Type { + return reflect.TypeOf((*T)(nil)).Elem() +} + +func (c Configurable[T]) cloneToReflectValuePtr() reflect.Value { + return reflect.ValueOf(c.clone()) +} + +func (c *Configurable[T]) clone() *Configurable[T] { + if c == nil { + return nil + } + var inner *appendWrapper[T] + if c.appendWrapper != nil { + inner = &appendWrapper[T]{} + if !c.appendWrapper.append.isEmpty() { + inner.append = *c.appendWrapper.append.clone() + } + } + + casesCopy := make(map[string]T, len(c.cases)) + for k, v := range c.cases { + casesCopy[k] = copyConfiguredValue(v) + } + + return &Configurable[T]{ + propertyName: c.propertyName, + typ: c.typ, + condition: c.condition, + cases: casesCopy, + appendWrapper: inner, + } +} + +func copyConfiguredValue[T ConfigurableElements](t T) T { + switch t2 := any(t).(type) { + case []string: + return any(slices.Clone(t2)).(T) + default: + return t + } +} diff --git a/proptools/extend.go b/proptools/extend.go index 63ff1d7..eec5d43 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -358,14 +358,21 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value switch srcFieldValue.Kind() { case reflect.Struct: - if sameTypes && dstFieldValue.Type() != srcFieldValue.Type() { - return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", - dstFieldValue.Type(), srcFieldValue.Type()) - } + if isConfigurable(srcField.Type) { + if srcFieldValue.Type() != dstFieldValue.Type() { + return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", + dstFieldValue.Type(), srcFieldValue.Type()) + } + } else { + if sameTypes && dstFieldValue.Type() != srcFieldValue.Type() { + return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", + dstFieldValue.Type(), srcFieldValue.Type()) + } - // Recursively extend the struct's fields. - recurse = append(recurse, dstFieldValue) - continue + // Recursively extend the struct's fields. + recurse = append(recurse, dstFieldValue) + continue + } case reflect.Bool, reflect.String, reflect.Slice, reflect.Map: if srcFieldValue.Type() != dstFieldValue.Type() { return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", @@ -433,6 +440,18 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { prepend := order == Prepend switch srcFieldValue.Kind() { + case reflect.Struct: + if !isConfigurable(srcFieldValue.Type()) { + panic("Should be unreachable") + } + if dstFieldValue.Interface().(configurableReflection).isEmpty() { + dstFieldValue.Set(srcFieldValue) + } else if prepend { + srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface()) + dstFieldValue.Set(srcFieldValue) + } else { + dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface()) + } case reflect.Bool: // Boolean OR dstFieldValue.Set(reflect.ValueOf(srcFieldValue.Bool() || dstFieldValue.Bool())) @@ -525,6 +544,19 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { // For append, replace the original value. dstFieldValue.Set(reflect.ValueOf(StringPtr(srcFieldValue.Elem().String()))) } + case reflect.Struct: + srcFieldValue := srcFieldValue.Elem() + if !isConfigurable(srcFieldValue.Type()) { + panic("Should be unreachable") + } + if dstFieldValue.Interface().(configurableReflection).isEmpty() { + dstFieldValue.Set(srcFieldValue) + } else if prepend { + srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface()) + dstFieldValue.Set(srcFieldValue) + } else { + dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface()) + } default: panic(fmt.Errorf("unexpected pointer kind %s", ptrKind)) } diff --git a/proptools/extend_test.go b/proptools/extend_test.go index e562776..61d3c40 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 { @@ -1254,6 +1256,91 @@ func appendPropertiesTestCases() []appendPropertyTestCase { }, err: extendPropertyErrorf("s", "filter error"), }, + { + name: "Append configurable", + dst: &struct{ S Configurable[[]string] }{ + S: Configurable[[]string]{ + typ: parser.SelectTypeSoongConfigVariable, + condition: "foo", + cases: map[string][]string{ + "a": {"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"}, + }, + appendWrapper: &appendWrapper[[]string]{}, + }, + }, + out: &struct{ S Configurable[[]string] }{ + S: Configurable[[]string]{ + typ: parser.SelectTypeSoongConfigVariable, + condition: "foo", + cases: map[string][]string{ + "a": {"1", "2"}, + }, + appendWrapper: &appendWrapper[[]string]{ + append: Configurable[[]string]{ + typ: parser.SelectTypeReleaseVariable, + condition: "bar", + cases: map[string][]string{ + "b": {"3", "4"}, + }, + appendWrapper: &appendWrapper[[]string]{}, + }, + }, + }, + }, + }, + { + name: "Prepend configurable", + order: Prepend, + dst: &struct{ S Configurable[[]string] }{ + S: Configurable[[]string]{ + typ: parser.SelectTypeSoongConfigVariable, + condition: "foo", + cases: map[string][]string{ + "a": {"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"}, + }, + appendWrapper: &appendWrapper[[]string]{}, + }, + }, + out: &struct{ S Configurable[[]string] }{ + S: Configurable[[]string]{ + typ: parser.SelectTypeReleaseVariable, + condition: "bar", + cases: map[string][]string{ + "b": {"3", "4"}, + }, + appendWrapper: &appendWrapper[[]string]{ + append: Configurable[[]string]{ + typ: parser.SelectTypeSoongConfigVariable, + condition: "foo", + cases: map[string][]string{ + "a": {"1", "2"}, + }, + appendWrapper: &appendWrapper[[]string]{}, + }, + }, + }, + }, + }, } } diff --git a/proptools/proptools.go b/proptools/proptools.go index 4580d01..98fe953 100644 --- a/proptools/proptools.go +++ b/proptools/proptools.go @@ -136,6 +136,14 @@ func isSliceOfStruct(t reflect.Type) bool { return isSlice(t) && isStruct(t.Elem()) } +func isStringOrStringPtr(t reflect.Type) bool { + return t.Kind() == reflect.String || (t.Kind() == reflect.Pointer && t.Elem().Kind() == reflect.String) +} + func isMapOfStruct(t reflect.Type) bool { return t.Kind() == reflect.Map && isStruct(t.Elem()) } + +func isConfigurable(t reflect.Type) bool { + return isStruct(t) && t.NumField() > 0 && typeFields(t)[0].Type == configurableMarkerType +} diff --git a/proptools/proptools_test.go b/proptools/proptools_test.go index 0fa0507..31057ee 100644 --- a/proptools/proptools_test.go +++ b/proptools/proptools_test.go @@ -15,6 +15,7 @@ package proptools import ( + "reflect" "testing" ) @@ -156,3 +157,42 @@ func TestClearField(t *testing.T) { t.Error("struct field is not cleared to zero.") } } + +func TestIsConfigurable(t *testing.T) { + testCases := []struct { + name string + value interface{} + expected bool + }{ + { + name: "Configurable string", + value: Configurable[string]{}, + expected: true, + }, + { + name: "Configurable string list", + value: Configurable[[]string]{}, + expected: true, + }, + { + name: "Configurable bool", + value: Configurable[bool]{}, + expected: true, + }, + { + name: "Other struct with a bool as the first field", + value: struct { + x bool + }{}, + expected: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + value := reflect.ValueOf(tc.value) + if isConfigurable(value.Type()) != tc.expected { + t.Errorf("Expected isConfigurable to return %t", tc.expected) + } + }) + } +} diff --git a/proptools/tag.go b/proptools/tag.go index 801fa3b..2f89249 100644 --- a/proptools/tag.go +++ b/proptools/tag.go @@ -56,7 +56,7 @@ func propertyIndexesWithTag(t reflect.Type, key, value string) [][]int { for i := 0; i < t.NumField(); i++ { field := t.Field(i) ft := field.Type - if isStruct(ft) || isStructPtr(ft) || isSliceOfStruct(ft) { + if (isStruct(ft) && !isConfigurable(ft)) || isStructPtr(ft) || isSliceOfStruct(ft) { if ft.Kind() == reflect.Ptr || ft.Kind() == reflect.Slice || ft.Kind() == reflect.Map { ft = ft.Elem() } diff --git a/proptools/unpack.go b/proptools/unpack.go index e6b688e..faef56a 100644 --- a/proptools/unpack.go +++ b/proptools/unpack.go @@ -172,7 +172,7 @@ func (ctx *unpackContext) buildPropertyMap(prefix string, properties []*parser.P continue } - itemProperties := make([]*parser.Property, len(propValue.Values), len(propValue.Values)) + itemProperties := make([]*parser.Property, len(propValue.Values)) for i, expr := range propValue.Values { itemProperties[i] = &parser.Property{ Name: property.Name + "[" + strconv.Itoa(i) + "]", @@ -301,7 +301,18 @@ func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect. continue } - if isStruct(fieldValue.Type()) { + if isConfigurable(fieldValue.Type()) { + // configurableType is the reflect.Type representation of a Configurable[whatever], + // while configuredType is the reflect.Type of the "whatever". + configurableType := fieldValue.Type() + configuredType := fieldValue.Interface().(configurableReflection).configuredType() + if unpackedValue, ok := ctx.unpackToConfigurable(propertyName, property, configurableType, configuredType); ok { + ExtendBasicType(fieldValue, unpackedValue, Append) + } + if len(ctx.errs) >= maxUnpackErrors { + return + } + } else if isStruct(fieldValue.Type()) { if property.Value.Eval().Type() != parser.MapType { ctx.addError(&UnpackError{ fmt.Errorf("can't assign %s value to map property %q", @@ -321,7 +332,6 @@ func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect. if len(ctx.errs) >= maxUnpackErrors { return } - } else { unpackedValue, err := propertyToValue(fieldValue.Type(), property) if err != nil && !ctx.addError(err) { @@ -332,16 +342,180 @@ func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect. } } +// Converts the given property to a pointer to a configurable struct +func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *parser.Property, configurableType, configuredType reflect.Type) (reflect.Value, bool) { + switch v := property.Value.(type) { + case *parser.String: + if configuredType.Kind() != reflect.String { + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign string value to configurable %s property %q", + configuredType.String(), property.Name), + property.Value.Pos(), + }) + return reflect.New(configurableType), false + } + result := Configurable[string]{ + propertyName: property.Name, + typ: parser.SelectTypeUnconfigured, + cases: map[string]string{ + default_select_branch_name: v.Value, + }, + appendWrapper: &appendWrapper[string]{}, + } + return reflect.ValueOf(&result), true + case *parser.Bool: + if configuredType.Kind() != reflect.Bool { + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign bool value to configurable %s property %q", + configuredType.String(), property.Name), + property.Value.Pos(), + }) + return reflect.New(configurableType), false + } + result := Configurable[bool]{ + propertyName: property.Name, + typ: parser.SelectTypeUnconfigured, + cases: map[string]bool{ + default_select_branch_name: v.Value, + }, + appendWrapper: &appendWrapper[bool]{}, + } + return reflect.ValueOf(&result), true + case *parser.List: + if configuredType.Kind() != reflect.Slice { + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign list value to configurable %s property %q", + configuredType.String(), property.Name), + property.Value.Pos(), + }) + return reflect.New(configurableType), false + } + switch configuredType.Elem().Kind() { + case reflect.String: + var cases map[string][]string + if v.Values != nil { + cases = map[string][]string{default_select_branch_name: make([]string, 0, len(v.Values))} + itemProperty := &parser.Property{NamePos: property.NamePos, ColonPos: property.ColonPos} + for i, expr := range v.Values { + itemProperty.Name = propertyName + "[" + strconv.Itoa(i) + "]" + itemProperty.Value = expr + exprUnpacked, err := propertyToValue(configuredType.Elem(), itemProperty) + if err != nil { + ctx.addError(err) + return reflect.ValueOf(Configurable[[]string]{}), false + } + cases[default_select_branch_name] = append(cases[default_select_branch_name], exprUnpacked.Interface().(string)) + } + } + result := Configurable[[]string]{ + propertyName: property.Name, + typ: parser.SelectTypeUnconfigured, + cases: cases, + appendWrapper: &appendWrapper[[]string]{}, + } + return reflect.ValueOf(&result), true + 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.Select: + resultPtr := reflect.New(configurableType) + result := resultPtr.Elem() + cases := reflect.MakeMapWithSize(reflect.MapOf(reflect.TypeOf(""), configuredType), len(v.Cases)) + for i, c := range v.Cases { + switch configuredType.Kind() { + case reflect.String, reflect.Bool: + p := &parser.Property{ + Name: property.Name + "[" + strconv.Itoa(i) + "]", + NamePos: c.ColonPos, + Value: c.Value, + } + val, err := propertyToValue(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") + } + p := &parser.Property{ + Name: property.Name + "[" + strconv.Itoa(i) + "]", + NamePos: c.ColonPos, + Value: c.Value, + } + val, ok := ctx.unpackToSlice(p.Name, p, configuredType) + if !ok { + return reflect.New(configurableType), false + } + cases.SetMapIndex(reflect.ValueOf(c.Pattern.Value), val) + default: + panic("This should be unreachable because ConfigurableElements only accepts strings, boools, or slices of strings") + } + } + resultPtr.Interface().(configurablePtrReflection).initialize( + property.Name, + v.Typ, + v.Condition.Value, + cases.Interface(), + ) + if v.Append != nil { + p := &parser.Property{ + Name: property.Name, + NamePos: property.NamePos, + Value: v.Append, + } + val, ok := ctx.unpackToConfigurable(propertyName, p, configurableType, configuredType) + if !ok { + return reflect.New(configurableType), false + } + result.Interface().(configurableReflection).setAppend(val.Elem().Interface()) + } + return resultPtr, true + default: + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign %s value to configurable %s property %q", + property.Value.Type(), configuredType.String(), property.Name), + property.Value.Pos(), + }) + return reflect.New(configurableType), false + } +} + +func (ctx *unpackContext) reportSelectOnNonConfigurablePropertyError( + property *parser.Property, +) bool { + if _, ok := property.Value.Eval().(*parser.Select); !ok { + return false + } + + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign select statement to non-configurable property %q. This requires a small soong change to enable in most cases, please file a go/soong-bug if you'd like to use a select statement here", + property.Name), + property.Value.Pos(), + }) + + return true +} + // unpackSlice creates a value of a given slice type from the property which should be a list func (ctx *unpackContext) unpackToSlice( sliceName string, property *parser.Property, sliceType reflect.Type) (reflect.Value, bool) { propValueAsList, ok := property.Value.Eval().(*parser.List) if !ok { - ctx.addError(&UnpackError{ - fmt.Errorf("can't assign %s value to list property %q", - property.Value.Type(), property.Name), - property.Value.Pos(), - }) + if !ctx.reportSelectOnNonConfigurablePropertyError(property) { + ctx.addError(&UnpackError{ + fmt.Errorf("can't assign %s value to list property %q", + property.Value.Type(), property.Name), + property.Value.Pos(), + }) + } return reflect.MakeSlice(sliceType, 0, 0), false } exprs := propValueAsList.Values diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go index 35aabf9..b51ed7a 100644 --- a/proptools/unpack_test.go +++ b/proptools/unpack_test.go @@ -721,6 +721,146 @@ var validUnpackTestCases = []struct { }, }, }, + { + name: "String configurable property that isn't configured", + input: ` + m { + foo: "bar" + } + `, + output: []interface{}{ + &struct { + Foo Configurable[string] + }{ + Foo: Configurable[string]{ + propertyName: "foo", + typ: parser.SelectTypeUnconfigured, + cases: map[string]string{ + default_select_branch_name: "bar", + }, + appendWrapper: &appendWrapper[string]{}, + }, + }, + }, + }, + { + name: "Bool configurable property that isn't configured", + input: ` + m { + foo: true, + } + `, + output: []interface{}{ + &struct { + Foo Configurable[bool] + }{ + Foo: Configurable[bool]{ + propertyName: "foo", + typ: parser.SelectTypeUnconfigured, + cases: map[string]bool{ + default_select_branch_name: true, + }, + appendWrapper: &appendWrapper[bool]{}, + }, + }, + }, + }, + { + name: "String list configurable property that isn't configured", + input: ` + m { + foo: ["a", "b"], + } + `, + output: []interface{}{ + &struct { + Foo Configurable[[]string] + }{ + Foo: Configurable[[]string]{ + propertyName: "foo", + typ: parser.SelectTypeUnconfigured, + cases: map[string][]string{ + default_select_branch_name: {"a", "b"}, + }, + appendWrapper: &appendWrapper[[]string]{}, + }, + }, + }, + }, + { + name: "Configurable property", + input: ` + m { + foo: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + "b": "b2", + _: "c2", + }) + } + `, + output: []interface{}{ + &struct { + Foo Configurable[string] + }{ + Foo: Configurable[string]{ + propertyName: "foo", + typ: parser.SelectTypeSoongConfigVariable, + condition: "my_namespace:my_variable", + cases: map[string]string{ + "a": "a2", + "b": "b2", + default_select_branch_name: "c2", + }, + appendWrapper: &appendWrapper[string]{}, + }, + }, + }, + }, + { + name: "Configurable property appending", + input: ` + m { + foo: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "a2", + "b": "b2", + _: "c2", + }) + select(soong_config_variable("my_namespace", "my_2nd_variable"), { + "d": "d2", + "e": "e2", + _: "f2", + }) + } + `, + output: []interface{}{ + &struct { + Foo Configurable[string] + }{ + Foo: Configurable[string]{ + propertyName: "foo", + typ: parser.SelectTypeSoongConfigVariable, + condition: "my_namespace:my_variable", + cases: map[string]string{ + "a": "a2", + "b": "b2", + default_select_branch_name: "c2", + }, + appendWrapper: &appendWrapper[string]{ + append: Configurable[string]{ + propertyName: "foo", + typ: parser.SelectTypeSoongConfigVariable, + condition: "my_namespace:my_2nd_variable", + cases: map[string]string{ + "d": "d2", + "e": "e2", + default_select_branch_name: "f2", + }, + appendWrapper: &appendWrapper[string]{}, + }, + }, + }, + }, + }, + }, } func TestUnpackProperties(t *testing.T) {