From a52b058cccd2caa778d0f97077adcd4ef7ffb68a Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 26 Apr 2024 16:39:54 -0700 Subject: Refactor selects In order to do less cloning, refactor selects so that all the soong-visibile structs are immutable to soong and can be reused. Additionally, refactor how the inner linked list of selects is managed, so that the append/prepend/replace logic is simpler. Bug: 323382414 Test: m nothing --no-skip-soong-tests Change-Id: Iba5d27405decc1b0596590c3e0555daeb044bf9e --- proptools/clone.go | 2 +- proptools/configurable.go | 326 ++++++++++++++++++++++++-------------- proptools/extend.go | 25 +-- proptools/extend_test.go | 390 +++++++++++++++++++++++++--------------------- proptools/unpack.go | 39 +++-- proptools/unpack_test.go | 192 ++++++++++++----------- 6 files changed, 557 insertions(+), 417 deletions(-) diff --git a/proptools/clone.go b/proptools/clone.go index 8ac5c6c..7606fa2 100644 --- a/proptools/clone.go +++ b/proptools/clone.go @@ -67,7 +67,7 @@ func copyProperties(dstValue, srcValue reflect.Value) { dstFieldValue.Set(srcFieldValue) case reflect.Struct: if isConfigurable(srcFieldValue.Type()) { - dstFieldValue.Set(srcFieldValue.Interface().(configurableReflection).cloneToReflectValuePtr().Elem()) + dstFieldValue.Set(reflect.ValueOf(srcFieldValue.Interface().(configurableReflection).clone())) } else { copyProperties(dstFieldValue, srcFieldValue) } diff --git a/proptools/configurable.go b/proptools/configurable.go index 2a094a3..06b39a5 100644 --- a/proptools/configurable.go +++ b/proptools/configurable.go @@ -36,18 +36,44 @@ type configurableMarker bool var configurableMarkerType reflect.Type = reflect.TypeOf((*configurableMarker)(nil)).Elem() +// ConfigurableCondition represents a condition that is being selected on, like +// arch(), os(), soong_config_variable("namespace", "variable"), or other variables. +// It's represented generically as a function name + arguments in blueprint, soong +// interprets the function name and args into specific variable values. +// +// ConfigurableCondition is treated as an immutable object so that it may be shared +// between different configurable properties. type ConfigurableCondition struct { - FunctionName string - Args []string + functionName string + args []string +} + +func NewConfigurableCondition(functionName string, args []string) ConfigurableCondition { + return ConfigurableCondition{ + functionName: functionName, + args: slices.Clone(args), + } +} + +func (c ConfigurableCondition) FunctionName() string { + return c.functionName +} + +func (c ConfigurableCondition) NumArgs() int { + return len(c.args) +} + +func (c ConfigurableCondition) Arg(i int) string { + return c.args[i] } func (c *ConfigurableCondition) String() string { var sb strings.Builder - sb.WriteString(c.FunctionName) + sb.WriteString(c.functionName) sb.WriteRune('(') - for i, arg := range c.Args { + for i, arg := range c.args { sb.WriteString(strconv.Quote(arg)) - if i < len(c.Args)-1 { + if i < len(c.args)-1 { sb.WriteString(", ") } } @@ -153,6 +179,16 @@ func (v *configurablePatternType) String() string { } } +// ConfigurablePattern represents a concrete value for a ConfigurableCase. +// Currently this just means the value of whatever variable is being looked +// up with the ConfigurableCase, but in the future it may be expanded to +// match multiple values (e.g. ranges of integers like 3..7). +// +// ConfigurablePattern can represent different types of values, like +// strings vs bools. +// +// ConfigurablePattern must be immutable so it can be shared between +// different configurable properties. type ConfigurablePattern struct { typ configurablePatternType stringValue string @@ -209,18 +245,17 @@ func (p *ConfigurablePattern) matchesValueType(v ConfigurableValue) bool { return p.typ == v.typ.patternType() } +// ConfigurableCase represents a set of ConfigurablePatterns +// (exactly 1 pattern per ConfigurableCase), and a value to use +// if all of the patterns are matched. +// +// ConfigurableCase must be immutable so it can be shared between +// different configurable properties. 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{}) } @@ -228,9 +263,11 @@ type configurableCaseReflection interface { var _ configurableCaseReflection = &ConfigurableCase[string]{} func NewConfigurableCase[T ConfigurableElements](patterns []ConfigurablePattern, value *T) ConfigurableCase[T] { + // Clone the values so they can't be modified from soong + patterns = slices.Clone(patterns) return ConfigurableCase[T]{ patterns: patterns, - value: value, + value: copyConfiguredValue(value), } } @@ -302,11 +339,22 @@ func configurableType(configuredType reflect.Type) (reflect.Type, error) { // 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 - conditions []ConfigurableCondition - cases []ConfigurableCase[T] - appendWrapper *appendWrapper[T] + marker configurableMarker + propertyName string + inner *configurableInner[T] +} + +type configurableInner[T ConfigurableElements] struct { + single singleConfigurable[T] + replace bool + next *configurableInner[T] +} + +// singleConfigurable must be immutable so it can be reused +// between multiple configurables +type singleConfigurable[T ConfigurableElements] struct { + conditions []ConfigurableCondition + cases []ConfigurableCase[T] } // Ignore the warning about the unused marker variable, it's used via reflection @@ -318,55 +366,61 @@ func NewConfigurable[T ConfigurableElements](conditions []ConfigurableCondition, panic(fmt.Sprintf("All configurables cases must have as many patterns as the configurable has conditions. Expected: %d, found: %d", len(conditions), len(c.patterns))) } } + // Clone the slices so they can't be modified from soong + conditions = slices.Clone(conditions) + cases = slices.Clone(cases) return Configurable[T]{ - conditions: conditions, - cases: cases, - appendWrapper: &appendWrapper[T]{}, + inner: &configurableInner[T]{ + single: singleConfigurable[T]{ + conditions: conditions, + cases: cases, + }, + }, } } -// 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] - replace bool -} - // Get returns the final value for the configurable property. // A configurable property may be unset, in which case Get will return nil. func (c *Configurable[T]) Get(evaluator ConfigurableEvaluator) *T { + result := c.inner.evaluate(c.propertyName, evaluator) + // Copy the result so that it can't be changed from soong + return copyConfiguredValue(result) +} + +// GetOrDefault is the same as Get, but will return the provided default value if the property was unset. +func (c *Configurable[T]) GetOrDefault(evaluator ConfigurableEvaluator, defaultValue T) T { + result := c.inner.evaluate(c.propertyName, evaluator) + if result != nil { + // Copy the result so that it can't be changed from soong + return copyAndDereferenceConfiguredValue(result) + } + return defaultValue +} + +func (c *configurableInner[T]) evaluate(propertyName string, evaluator ConfigurableEvaluator) *T { if c == nil { return nil } - if c.appendWrapper == nil { - return c.evaluateNonTransitive(evaluator) + if c.next == nil { + return c.single.evaluateNonTransitive(propertyName, evaluator) } - if c.appendWrapper.replace { + if c.replace { return replaceConfiguredValues( - c.evaluateNonTransitive(evaluator), - c.appendWrapper.append.Get(evaluator), + c.single.evaluateNonTransitive(propertyName, evaluator), + c.next.evaluate(propertyName, evaluator), ) } else { return appendConfiguredValues( - c.evaluateNonTransitive(evaluator), - c.appendWrapper.append.Get(evaluator), + c.single.evaluateNonTransitive(propertyName, evaluator), + c.next.evaluate(propertyName, evaluator), ) } } -// GetOrDefault is the same as Get, but will return the provided default value if the property was unset. -func (c *Configurable[T]) GetOrDefault(evaluator ConfigurableEvaluator, defaultValue T) T { - result := c.Get(evaluator) - if result != nil { - return *result - } - return defaultValue -} - -func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) *T { +func (c *singleConfigurable[T]) evaluateNonTransitive(propertyName string, evaluator ConfigurableEvaluator) *T { 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)) + evaluator.PropertyErrorf(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 } } @@ -376,13 +430,13 @@ func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) } 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)) + evaluator.PropertyErrorf(propertyName, "Expected 0 or 1 branches in an unconfigured select, found %d", len(c.cases)) return nil } } values := make([]ConfigurableValue, len(c.conditions)) for i, condition := range c.conditions { - values[i] = evaluator.EvaluateConfiguration(condition, c.propertyName) + values[i] = evaluator.EvaluateConfiguration(condition, propertyName) } foundMatch := false var result *T @@ -390,7 +444,7 @@ func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) 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()) + evaluator.PropertyErrorf(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]) { @@ -406,7 +460,7 @@ func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) if foundMatch { return result } - evaluator.PropertyErrorf(c.propertyName, "%s had value %s, which was not handled by the select statement", c.conditions, values) + evaluator.PropertyErrorf(propertyName, "%s had value %s, which was not handled by the select statement", c.conditions, values) return nil } @@ -468,9 +522,9 @@ func replaceConfiguredValues[T ConfigurableElements](a, b *T) *T { // 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, replace bool) + setAppend(append any, replace bool, prepend bool) configuredType() reflect.Type - cloneToReflectValuePtr() reflect.Value + clone() any isEmpty() bool } @@ -485,44 +539,120 @@ var _ configurablePtrReflection = &Configurable[string]{} func (c *Configurable[T]) initialize(propertyName string, conditions []ConfigurableCondition, cases any) { c.propertyName = propertyName - c.conditions = conditions - c.cases = cases.([]ConfigurableCase[T]) - c.appendWrapper = &appendWrapper[T]{} + c.inner = &configurableInner[T]{ + single: singleConfigurable[T]{ + conditions: conditions, + cases: cases.([]ConfigurableCase[T]), + }, + } +} + +func (c Configurable[T]) setAppend(append any, replace bool, prepend bool) { + a := append.(Configurable[T]) + if a.inner.isEmpty() { + return + } + c.inner.setAppend(a.inner, replace, prepend) + if c.inner == c.inner.next { + panic("pointer loop") + } } -func (c Configurable[T]) setAppend(append any, replace bool) { - // TODO(b/323382414) Update the propertyName of appended selects - if c.appendWrapper.append.isEmpty() { - x := append.(Configurable[T]) - c.appendWrapper.append = *(&x).clone() - c.appendWrapper.replace = replace +func (c *configurableInner[T]) setAppend(append *configurableInner[T], replace bool, prepend bool) { + if c.isEmpty() { + *c = *append.clone() + } else if prepend { + if replace && c.alwaysHasValue() { + // The current value would always override the prepended value, so don't do anything + return + } + // We're going to replace the head node with the one from append, so allocate + // a new one here. + old := &configurableInner[T]{ + single: c.single, + replace: c.replace, + next: c.next, + } + *c = *append.clone() + curr := c + for curr.next != nil { + curr = curr.next + } + curr.next = old + curr.replace = replace } else { // If we're replacing with something that always has a value set, // we can optimize the code by replacing our entire append chain here. - if replace && append.(Configurable[T]).alwaysHasValue() { - x := append.(Configurable[T]) - c.appendWrapper.append = *(&x).clone() - c.appendWrapper.replace = replace + if replace && append.alwaysHasValue() { + *c = *append.clone() } else { - c.appendWrapper.append.setAppend(append, replace) + curr := c + for curr.next != nil { + curr = curr.next + } + curr.next = append.clone() + curr.replace = replace } } } +func (c Configurable[T]) clone() any { + return Configurable[T]{ + propertyName: c.propertyName, + inner: c.inner.clone(), + } +} + +func (c *configurableInner[T]) clone() *configurableInner[T] { + if c == nil { + return nil + } + return &configurableInner[T]{ + // We don't need to clone the singleConfigurable because + // it's supposed to be immutable + single: c.single, + replace: c.replace, + next: c.next.clone(), + } +} + +func (c *configurableInner[T]) isEmpty() bool { + if c == nil { + return true + } + if !c.single.isEmpty() { + return false + } + return c.next.isEmpty() +} + func (c Configurable[T]) isEmpty() bool { + return c.inner.isEmpty() +} + +func (c *singleConfigurable[T]) isEmpty() bool { + if c == nil { + return true + } if len(c.cases) > 1 { return false } if len(c.cases) == 1 && c.cases[0].value != nil { return false } - if c.appendWrapper != nil { - return c.appendWrapper.append.isEmpty() - } return true } -func (c Configurable[T]) alwaysHasValue() bool { +func (c *configurableInner[T]) alwaysHasValue() bool { + for curr := c; curr != nil; curr = curr.next { + if curr.single.alwaysHasValue() { + return true + } + } + return false +} + +func (c *singleConfigurable[T]) alwaysHasValue() bool { if len(c.cases) == 0 { return false } @@ -531,11 +661,6 @@ func (c Configurable[T]) alwaysHasValue() bool { return false } } - - if c.appendWrapper != nil && !c.appendWrapper.append.isEmpty() { - return c.appendWrapper.append.alwaysHasValue() - } - return true } @@ -543,45 +668,6 @@ 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() - inner.replace = c.appendWrapper.replace - } - } - - var conditionsCopy []ConfigurableCondition - if c.conditions != nil { - conditionsCopy = make([]ConfigurableCondition, len(c.conditions)) - copy(conditionsCopy, c.conditions) - } - - var casesCopy []ConfigurableCase[T] - if c.cases != nil { - casesCopy = make([]ConfigurableCase[T], len(c.cases)) - for i, case_ := range c.cases { - casesCopy[i] = case_.Clone() - } - } - - return &Configurable[T]{ - propertyName: c.propertyName, - conditions: conditionsCopy, - cases: casesCopy, - appendWrapper: inner, - } -} - func copyConfiguredValue[T ConfigurableElements](t *T) *T { if t == nil { return nil @@ -591,6 +677,16 @@ func copyConfiguredValue[T ConfigurableElements](t *T) *T { result := any(slices.Clone(t2)).(T) return &result default: - return t + x := *t + return &x + } +} + +func copyAndDereferenceConfiguredValue[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 dc5a145..ec25d51 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -507,26 +507,15 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { if !isConfigurable(srcFieldValue.Type()) { panic("Should be unreachable") } - unpackedSrc := srcFieldValue.Interface().(configurableReflection) + replace := order == Prepend_replace || order == Replace unpackedDst := dstFieldValue.Interface().(configurableReflection) - if unpackedSrc.isEmpty() { - // Do nothing - } else if unpackedDst.isEmpty() { - dstFieldValue.Set(unpackedSrc.cloneToReflectValuePtr().Elem()) - } else if order == Prepend { - clonedSrc := unpackedSrc.cloneToReflectValuePtr().Elem() - clonedSrc.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), false) - dstFieldValue.Set(clonedSrc) - } else if order == Append { - unpackedDst.setAppend(srcFieldValue.Interface(), false) - } else if order == Replace { - unpackedDst.setAppend(srcFieldValue.Interface(), true) - } else if order == Prepend_replace { - clonedSrc := unpackedSrc.cloneToReflectValuePtr().Elem() - clonedSrc.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), true) - dstFieldValue.Set(clonedSrc) + if unpackedDst.isEmpty() { + // Properties that were never initialized via unpacking from a bp file value + // will have a nil inner value, making them unable to be modified without a pointer + // like we don't have here. So instead replace the whole configurable object. + dstFieldValue.Set(reflect.ValueOf(srcFieldValue.Interface().(configurableReflection).clone())) } else { - panic(fmt.Sprintf("Unexpected order: %d", order)) + unpackedDst.setAppend(srcFieldValue.Interface(), replace, prepend) } case reflect.Bool: // Boolean OR diff --git a/proptools/extend_test.go b/proptools/extend_test.go index 71a0bc2..13fd04f 100644 --- a/proptools/extend_test.go +++ b/proptools/extend_test.go @@ -1258,73 +1258,82 @@ func appendPropertiesTestCases() []appendPropertyTestCase { name: "Append configurable", dst: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, }, - }}, - cases: []ConfigurableCase[[]string]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: &[]string{"1", "2"}, - }}, - appendWrapper: &appendWrapper[[]string]{}, + }, }, }, src: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - conditions: []ConfigurableCondition{{ - FunctionName: "release_variable", - Args: []string{ - "bar", + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "release_variable", + args: []string{ + "bar", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, }, - }}, - cases: []ConfigurableCase[[]string]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "b", - }}, - value: &[]string{"3", "4"}, - }}, - appendWrapper: &appendWrapper[[]string]{}, + }, }, }, out: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - 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]{ + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ conditions: []ConfigurableCondition{{ - FunctionName: "release_variable", - Args: []string{ - "bar", + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", }, }}, cases: []ConfigurableCase[[]string]{{ patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, - stringValue: "b", + stringValue: "a", }}, - value: &[]string{"3", "4"}, + value: &[]string{"1", "2"}, }}, - appendWrapper: &appendWrapper[[]string]{}, + }, + next: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "release_variable", + args: []string{ + "bar", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, + }, }, }, }, @@ -1335,73 +1344,82 @@ func appendPropertiesTestCases() []appendPropertyTestCase { order: Prepend, dst: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, }, - }}, - cases: []ConfigurableCase[[]string]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: &[]string{"1", "2"}, - }}, - appendWrapper: &appendWrapper[[]string]{}, + }, }, }, src: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - conditions: []ConfigurableCondition{{ - FunctionName: "release_variable", - Args: []string{ - "bar", + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "release_variable", + args: []string{ + "bar", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "b", + }}, + value: &[]string{"3", "4"}, + }}, }, - }}, - cases: []ConfigurableCase[[]string]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "b", - }}, - value: &[]string{"3", "4"}, - }}, - appendWrapper: &appendWrapper[[]string]{}, + }, }, }, out: &struct{ S Configurable[[]string] }{ S: Configurable[[]string]{ - 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]{ + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", + functionName: "release_variable", + args: []string{ + "bar", }, }}, cases: []ConfigurableCase[[]string]{{ patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, - stringValue: "a", + stringValue: "b", }}, - value: &[]string{"1", "2"}, + value: &[]string{"3", "4"}, }}, - appendWrapper: &appendWrapper[[]string]{}, + }, + next: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[[]string]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: &[]string{"1", "2"}, + }}, + }, }, }, }, @@ -1875,26 +1893,29 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { dst: []interface{}{ &struct{ S Configurable[bool] }{ S: Configurable[bool]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, }, - }}, - cases: []ConfigurableCase[bool]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: BoolPtr(true), - }, { - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeDefault, - }}, - value: BoolPtr(false), - }}, - appendWrapper: &appendWrapper[bool]{}, + }, }, }, }, @@ -1904,31 +1925,34 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { out: []interface{}{ &struct{ S Configurable[bool] }{ S: Configurable[bool]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", - }, - }}, - cases: []ConfigurableCase[bool]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: BoolPtr(true), - }, { - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeDefault, - }}, - value: BoolPtr(false), - }}, - appendWrapper: &appendWrapper[bool]{ - append: Configurable[bool]{ + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), }}, - appendWrapper: &appendWrapper[bool]{}, + }, + next: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + }, }, }, }, @@ -1941,26 +1965,29 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { dst: []interface{}{ &struct{ S Configurable[bool] }{ S: Configurable[bool]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, + cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, + value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), + }}, }, - }}, - cases: []ConfigurableCase[bool]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: BoolPtr(true), - }, { - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeDefault, - }}, - value: BoolPtr(false), - }}, - appendWrapper: &appendWrapper[bool]{}, + }, }, }, }, @@ -1970,31 +1997,34 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { out: []interface{}{ &struct{ S Configurable[bool] }{ S: Configurable[bool]{ - conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ - "my_namespace", - "foo", - }, - }}, - cases: []ConfigurableCase[bool]{{ - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeString, - stringValue: "a", - }}, - value: BoolPtr(true), - }, { - patterns: []ConfigurablePattern{{ - typ: configurablePatternTypeDefault, - }}, - value: BoolPtr(false), - }}, - appendWrapper: &appendWrapper[bool]{ - append: Configurable[bool]{ + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "foo", + }, + }}, cases: []ConfigurableCase[bool]{{ + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeString, + stringValue: "a", + }}, value: BoolPtr(true), + }, { + patterns: []ConfigurablePattern{{ + typ: configurablePatternTypeDefault, + }}, + value: BoolPtr(false), }}, - appendWrapper: &appendWrapper[bool]{}, + }, + next: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + }, }, }, }, diff --git a/proptools/unpack.go b/proptools/unpack.go index 0cddcdb..dd0f119 100644 --- a/proptools/unpack.go +++ b/proptools/unpack.go @@ -356,10 +356,13 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } result := Configurable[string]{ propertyName: property.Name, - cases: []ConfigurableCase[string]{{ - value: &v.Value, - }}, - appendWrapper: &appendWrapper[string]{}, + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ + cases: []ConfigurableCase[string]{{ + value: &v.Value, + }}, + }, + }, } return reflect.ValueOf(&result), true case *parser.Bool: @@ -373,10 +376,13 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } result := Configurable[bool]{ propertyName: property.Name, - cases: []ConfigurableCase[bool]{{ - value: &v.Value, - }}, - appendWrapper: &appendWrapper[bool]{}, + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: &v.Value, + }}, + }, + }, } return reflect.ValueOf(&result), true case *parser.List: @@ -407,10 +413,13 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa } result := Configurable[[]string]{ propertyName: property.Name, - cases: []ConfigurableCase[[]string]{{ - value: &value, - }}, - appendWrapper: &appendWrapper[[]string]{}, + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + cases: []ConfigurableCase[[]string]{{ + value: &value, + }}, + }, + }, } return reflect.ValueOf(&result), true default: @@ -432,8 +441,8 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa args[j] = arg.Value } conditions[i] = ConfigurableCondition{ - FunctionName: cond.FunctionName, - Args: args, + functionName: cond.FunctionName, + args: args, } } @@ -512,7 +521,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa if !ok { return reflect.New(configurableType), false } - result.Interface().(configurableReflection).setAppend(val.Elem().Interface(), false) + result.Interface().(configurableReflection).setAppend(val.Elem().Interface(), false, false) } return resultPtr, true default: diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go index 009c65b..5e333b6 100644 --- a/proptools/unpack_test.go +++ b/proptools/unpack_test.go @@ -734,10 +734,13 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - cases: []ConfigurableCase[string]{{ - value: StringPtr("bar"), - }}, - appendWrapper: &appendWrapper[string]{}, + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ + cases: []ConfigurableCase[string]{{ + value: StringPtr("bar"), + }}, + }, + }, }, }, }, @@ -755,10 +758,13 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[bool]{ propertyName: "foo", - cases: []ConfigurableCase[bool]{{ - value: BoolPtr(true), - }}, - appendWrapper: &appendWrapper[bool]{}, + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + }, + }, }, }, }, @@ -776,10 +782,13 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[[]string]{ propertyName: "foo", - cases: []ConfigurableCase[[]string]{{ - value: &[]string{"a", "b"}, - }}, - appendWrapper: &appendWrapper[[]string]{}, + inner: &configurableInner[[]string]{ + single: singleConfigurable[[]string]{ + cases: []ConfigurableCase[[]string]{{ + value: &[]string{"a", "b"}, + }}, + }, + }, }, }, }, @@ -801,36 +810,39 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - 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, + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ + conditions: []ConfigurableCondition{{ + functionName: "soong_config_variable", + args: []string{ + "my_namespace", + "my_variable", + }, }}, - value: StringPtr("c2"), + 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]{}, }, }, }, @@ -856,68 +868,70 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - 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", + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ conditions: []ConfigurableCondition{{ - FunctionName: "soong_config_variable", - Args: []string{ + functionName: "soong_config_variable", + args: []string{ "my_namespace", - "my_2nd_variable", + "my_variable", }, }}, cases: []ConfigurableCase[string]{ { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, - stringValue: "d", + stringValue: "a", }}, - value: StringPtr("d2"), + value: StringPtr("a2"), }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeString, - stringValue: "e", + stringValue: "b", }}, - value: StringPtr("e2"), + value: StringPtr("b2"), }, { patterns: []ConfigurablePattern{{ typ: configurablePatternTypeDefault, }}, - value: StringPtr("f2"), + value: StringPtr("c2"), + }, + }, + }, + next: &configurableInner[string]{ + single: singleConfigurable[string]{ + 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]{}, }, }, }, @@ -941,21 +955,23 @@ var validUnpackTestCases = []struct { }{ Foo: Configurable[string]{ propertyName: "foo", - cases: []ConfigurableCase[string]{ - { - value: StringPtr("asdf"), + inner: &configurableInner[string]{ + single: singleConfigurable[string]{ + cases: []ConfigurableCase[string]{{ + value: StringPtr("asdf"), + }}, }, }, - appendWrapper: &appendWrapper[string]{}, }, Bar: Configurable[bool]{ propertyName: "bar", - cases: []ConfigurableCase[bool]{ - { - value: BoolPtr(true), + inner: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, }, }, - appendWrapper: &appendWrapper[bool]{}, }, }, }, -- cgit v1.2.3