diff options
author | Treehugger Robot <android-test-infra-autosubmit@system.gserviceaccount.com> | 2024-04-30 01:15:42 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2024-04-30 01:15:42 +0000 |
commit | ca5ffdd3ce32163c8e957f2b487af3dd735853cf (patch) | |
tree | de9462afc063e55dde1689236c80aa832d4189e6 | |
parent | c10b49b5e914a3deb86fd4226813204a564e32b5 (diff) | |
parent | a52b058cccd2caa778d0f97077adcd4ef7ffb68a (diff) | |
download | blueprint-ca5ffdd3ce32163c8e957f2b487af3dd735853cf.tar.gz |
Merge changes from topics "fix_selects_appending", "refactor_selects" into main
* changes:
Refactor selects
Fix bugs when appending selects
Allow extending configurable propeties with non-configurable properties
-rw-r--r-- | proptools/clone.go | 2 | ||||
-rw-r--r-- | proptools/configurable.go | 332 | ||||
-rw-r--r-- | proptools/extend.go | 70 | ||||
-rw-r--r-- | proptools/extend_test.go | 358 | ||||
-rw-r--r-- | proptools/typeequal.go | 8 | ||||
-rw-r--r-- | proptools/unpack.go | 39 | ||||
-rw-r--r-- | proptools/unpack_test.go | 192 |
7 files changed, 696 insertions, 305 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 f521ab4..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), } } @@ -257,6 +294,24 @@ func configurableCaseType(configuredType reflect.Type) reflect.Type { panic("unimplemented") } +// for the given T, return the reflect.type of Configurable[T] +func configurableType(configuredType reflect.Type) (reflect.Type, error) { + // 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(Configurable[string]{}), nil + case reflect.Bool: + return reflect.TypeOf(Configurable[bool]{}), nil + case reflect.Slice: + switch configuredType.Elem().Kind() { + case reflect.String: + return reflect.TypeOf(Configurable[[]string]{}), nil + } + } + return nil, fmt.Errorf("configurable structs can only contain strings, bools, or string slices, found %s", configuredType.String()) +} + // 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: @@ -284,11 +339,22 @@ func configurableCaseType(configuredType reflect.Type) reflect.Type { // 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 @@ -300,52 +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 { - if c == nil || c.appendWrapper == nil { + 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.replace { + if c.next == nil { + return c.single.evaluateNonTransitive(propertyName, evaluator) + } + 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 } } @@ -355,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 @@ -369,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]) { @@ -385,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 } @@ -447,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 } @@ -464,62 +539,133 @@ 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) { - if c.appendWrapper.append.isEmpty() { - c.appendWrapper.append = append.(Configurable[T]) - c.appendWrapper.replace = replace - } else { - c.appendWrapper.append.setAppend(append, replace) +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]) isEmpty() bool { - if c.appendWrapper != nil && !c.appendWrapper.append.isEmpty() { - return false +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.alwaysHasValue() { + *c = *append.clone() + } else { + curr := c + for curr.next != nil { + curr = curr.next + } + curr.next = append.clone() + curr.replace = replace + } } - return len(c.cases) == 0 } -func (c Configurable[T]) configuredType() reflect.Type { - return reflect.TypeOf((*T)(nil)).Elem() +func (c Configurable[T]) clone() any { + return Configurable[T]{ + propertyName: c.propertyName, + inner: c.inner.clone(), + } } -func (c Configurable[T]) cloneToReflectValuePtr() reflect.Value { - return reflect.ValueOf(c.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 *Configurable[T]) clone() *Configurable[T] { +func (c *configurableInner[T]) isEmpty() bool { if c == nil { - return nil + return true } - 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 - } + if !c.single.isEmpty() { + return false } + return c.next.isEmpty() +} - conditionsCopy := make([]ConfigurableCondition, len(c.conditions)) - copy(conditionsCopy, c.conditions) +func (c Configurable[T]) isEmpty() bool { + return c.inner.isEmpty() +} - casesCopy := make([]ConfigurableCase[T], len(c.cases)) - for i, case_ := range c.cases { - casesCopy[i] = case_.Clone() +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 + } + return true +} + +func (c *configurableInner[T]) alwaysHasValue() bool { + for curr := c; curr != nil; curr = curr.next { + if curr.single.alwaysHasValue() { + return true + } + } + return false +} - return &Configurable[T]{ - propertyName: c.propertyName, - conditions: conditionsCopy, - cases: casesCopy, - appendWrapper: inner, +func (c *singleConfigurable[T]) alwaysHasValue() bool { + if len(c.cases) == 0 { + return false + } + for _, c := range c.cases { + if c.value == nil { + return false + } } + return true +} + +func (c Configurable[T]) configuredType() reflect.Type { + return reflect.TypeOf((*T)(nil)).Elem() } func copyConfiguredValue[T ConfigurableElements](t *T) *T { @@ -531,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 110fb24..ec25d51 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -384,12 +384,16 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value continue } case reflect.Bool, reflect.String, reflect.Slice, reflect.Map: - if srcFieldValue.Type() != dstFieldValue.Type() { + // If the types don't match or srcFieldValue cannot be converted to a Configurable type, it's an error + ct, err := configurableType(srcFieldValue.Type()) + if srcFieldValue.Type() != dstFieldValue.Type() && (err != nil || dstFieldValue.Type() != ct) { return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", dstFieldValue.Type(), srcFieldValue.Type()) } case reflect.Ptr: - if srcFieldValue.Type() != dstFieldValue.Type() { + // If the types don't match or srcFieldValue cannot be converted to a Configurable type, it's an error + ct, err := configurableType(srcFieldValue.Type().Elem()) + if srcFieldValue.Type() != dstFieldValue.Type() && (err != nil || dstFieldValue.Type() != ct) { return extendPropertyErrorf(propertyName(srcField), "mismatched types %s and %s", dstFieldValue.Type(), srcFieldValue.Type()) } @@ -457,25 +461,61 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) { prepend := order == Prepend || order == Prepend_replace + if !srcFieldValue.IsValid() { + return + } + + // If dst is a Configurable and src isn't, promote src to a Configurable. + // This isn't necessary if all property structs are using Configurable values, + // but it's helpful to avoid having to change as many places in the code when + // converting properties to Configurable properties. For example, load hooks + // make their own mini-property structs and append them onto the main property + // structs when they want to change the default values of properties. + srcFieldType := srcFieldValue.Type() + if isConfigurable(dstFieldValue.Type()) && !isConfigurable(srcFieldType) { + var value reflect.Value + if srcFieldType.Kind() == reflect.Pointer { + srcFieldType = srcFieldType.Elem() + if srcFieldValue.IsNil() { + value = srcFieldValue + } else { + // Copy the pointer + value = reflect.New(srcFieldType) + value.Elem().Set(srcFieldValue.Elem()) + } + } else { + value = reflect.New(srcFieldType) + value.Elem().Set(srcFieldValue) + } + caseType := configurableCaseType(srcFieldType) + case_ := reflect.New(caseType) + case_.Interface().(configurableCaseReflection).initialize(nil, value.Interface()) + cases := reflect.MakeSlice(reflect.SliceOf(caseType), 0, 1) + cases = reflect.Append(cases, case_.Elem()) + ct, err := configurableType(srcFieldType) + if err != nil { + // Should be unreachable due to earlier checks + panic(err.Error()) + } + temp := reflect.New(ct) + temp.Interface().(configurablePtrReflection).initialize("", nil, cases.Interface()) + srcFieldValue = temp.Elem() + } + switch srcFieldValue.Kind() { case reflect.Struct: if !isConfigurable(srcFieldValue.Type()) { panic("Should be unreachable") } - if dstFieldValue.Interface().(configurableReflection).isEmpty() { - dstFieldValue.Set(srcFieldValue) - } else if order == Prepend { - srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), false) - dstFieldValue.Set(srcFieldValue) - } else if order == Append { - dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface(), false) - } else if order == Replace { - dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface(), true) - } else if order == Prepend_replace { - srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), true) - dstFieldValue.Set(srcFieldValue) + replace := order == Prepend_replace || order == Replace + unpackedDst := dstFieldValue.Interface().(configurableReflection) + 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 b2920f5..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"}, + }}, + }, }, }, }, @@ -1869,6 +1887,150 @@ func appendMatchingPropertiesTestCases() []appendMatchingPropertiesTestCase { }, err: extendPropertyErrorf("s", "mismatched types []int and []string"), }, + { + name: "Append *bool to Configurable[bool]", + order: Append, + dst: []interface{}{ + &struct{ S Configurable[bool] }{ + S: 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), + }}, + }, + }, + }, + }, + }, + src: &struct{ S *bool }{ + S: BoolPtr(true), + }, + out: []interface{}{ + &struct{ S Configurable[bool] }{ + S: 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), + }}, + }, + next: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + }, + }, + }, + }, + }, + }, + }, + { + name: "Append bool to Configurable[bool]", + order: Append, + dst: []interface{}{ + &struct{ S Configurable[bool] }{ + S: 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), + }}, + }, + }, + }, + }, + }, + src: &struct{ S bool }{ + S: true, + }, + out: []interface{}{ + &struct{ S Configurable[bool] }{ + S: 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), + }}, + }, + next: &configurableInner[bool]{ + single: singleConfigurable[bool]{ + cases: []ConfigurableCase[bool]{{ + value: BoolPtr(true), + }}, + }, + }, + }, + }, + }, + }, + }, } } diff --git a/proptools/typeequal.go b/proptools/typeequal.go index d9b3c18..3fadae4 100644 --- a/proptools/typeequal.go +++ b/proptools/typeequal.go @@ -62,6 +62,10 @@ func typeEqual(v1, v2 reflect.Value) bool { return true } + if isConfigurable(v1.Type()) { + return true + } + for i := 0; i < v1.NumField(); i++ { v1 := v1.Field(i) v2 := v2.Field(i) @@ -94,6 +98,10 @@ func concreteType(v reflect.Value) bool { return true } + if isConfigurable(v.Type()) { + return true + } + for i := 0; i < v.NumField(); i++ { v := v.Field(i) 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]{}, }, }, }, |