Filter product variable property structs

For each module, take the default product variable properties and
create a new type that contains only the properties that exist in
the current module.  This will provide better errors when attemping
to set product variable properties that will never get used for
a module, and fixes errors when a module attempts to use a product
variable that also contains properties that don't apply to the
current module.

Fixes: 79249983
Test: TestProductVariables
Test: m checkbuild
Change-Id: I4dc24f7781b8ce798651cfc8bf3563005a92c13d

android/module.go

@@ -510,8 +510,19 @@ func InitAndroidModule(m Module) {
 
 	m.AddProperties(
 		&base.nameProperties,
-		&base.commonProperties,
-		&base.variableProperties)
+		&base.commonProperties)
+
+	// Allow tests to override the default product variables
+	if base.variableProperties == nil {
+		base.variableProperties = zeroProductVariables
+	}
+
+	// Filter the product variables properties to the ones that exist on this module
+	base.variableProperties = createVariableProperties(m.GetProperties(), base.variableProperties)
+	if base.variableProperties != nil {
+		m.AddProperties(base.variableProperties)
+	}
+
 	base.generalProperties = m.GetProperties()
 	base.customizableProperties = m.GetProperties()
 
@@ -593,7 +604,7 @@ type ModuleBase struct {
 
 	nameProperties          nameProperties
 	commonProperties        commonProperties
-	variableProperties      variableProperties
+	variableProperties      interface{}
 	hostAndDeviceProperties hostAndDeviceProperties
 	generalProperties       []interface{}
 	archProperties          [][]interface{}

android/mutator.go

@@ -15,6 +15,8 @@
 package android
 
 import (
+	"reflect"
+
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/proptools"
 )
@@ -244,8 +246,23 @@ func (t *topDownMutatorContext) Rename(name string) {
 }
 
 func (t *topDownMutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module {
-	inherited := []interface{}{&t.Module().base().commonProperties, &t.Module().base().variableProperties}
+	inherited := []interface{}{&t.Module().base().commonProperties}
 	module := t.bp.CreateModule(ModuleFactoryAdaptor(factory), append(inherited, props...)...).(Module)
+
+	if t.Module().base().variableProperties != nil && module.base().variableProperties != nil {
+		src := t.Module().base().variableProperties
+		dst := []interface{}{
+			module.base().variableProperties,
+			// Put an empty copy of the src properties into dst so that properties in src that are not in dst
+			// don't cause a "failed to find property to extend" error.
+			proptools.CloneEmptyProperties(reflect.ValueOf(src).Elem()).Interface(),
+		}
+		err := proptools.AppendMatchingProperties(dst, src, nil)
+		if err != nil {
+			panic(err)
+		}
+	}
+
 	return module
 }
 

android/variable.go

@@ -122,7 +122,7 @@ type variableProperties struct {
 	} `android:"arch_variant"`
 }
 
-var zeroProductVariables variableProperties
+var zeroProductVariables interface{} = variableProperties{}
 
 type productVariables struct {
 	// Suffix to add to generated Makefiles
@@ -366,8 +366,13 @@ func variableMutator(mctx BottomUpMutatorContext) {
 
 	// TODO: depend on config variable, create variants, propagate variants up tree
 	a := module.base()
-	variableValues := reflect.ValueOf(&a.variableProperties.Product_variables).Elem()
-	zeroValues := reflect.ValueOf(zeroProductVariables.Product_variables)
+
+	if a.variableProperties == nil {
+		return
+	}
+
+	variableValues := reflect.ValueOf(a.variableProperties).Elem().FieldByName("Product_variables")
+	zeroValues := reflect.ValueOf(zeroProductVariables).FieldByName("Product_variables")
 
 	for i := 0; i < variableValues.NumField(); i++ {
 		variableValue := variableValues.Field(i)
@@ -496,3 +501,106 @@ func printfIntoProperty(propertyValue reflect.Value, variableValue interface{})
 
 	return nil
 }
+
+var variablePropTypeMap OncePer
+
+// sliceToTypeArray takes a slice of property structs and returns a reflection created array containing the
+// reflect.Types of each property struct.  The result can be used as a key in a map.
+func sliceToTypeArray(s []interface{}) interface{} {
+	// Create an array using reflection whose length is the length of the input slice
+	ret := reflect.New(reflect.ArrayOf(len(s), reflect.TypeOf(reflect.TypeOf(0)))).Elem()
+	for i, e := range s {
+		ret.Index(i).Set(reflect.ValueOf(reflect.TypeOf(e)))
+	}
+	return ret.Interface()
+}
+
+// createVariableProperties takes the list of property structs for a module and returns a property struct that
+// contains the product variable properties that exist in the property structs, or nil if there are none.  It
+// caches the result.
+func createVariableProperties(moduleTypeProps []interface{}, productVariables interface{}) interface{} {
+	// Convert the moduleTypeProps to an array of reflect.Types that can be used as a key in the OncePer.
+	key := sliceToTypeArray(moduleTypeProps)
+
+	// Use the variablePropTypeMap OncePer to cache the result for each set of property struct types.
+	typ, _ := variablePropTypeMap.Once(NewCustomOnceKey(key), func() interface{} {
+		// Compute the filtered property struct type.
+		return createVariablePropertiesType(moduleTypeProps, productVariables)
+	}).(reflect.Type)
+
+	if typ == nil {
+		return nil
+	}
+
+	// Create a new pointer to a filtered property struct.
+	return reflect.New(typ).Interface()
+}
+
+// createVariablePropertiesType creates a new type that contains only the product variable properties that exist in
+// a list of property structs.
+func createVariablePropertiesType(moduleTypeProps []interface{}, productVariables interface{}) reflect.Type {
+	typ, _ := proptools.FilterPropertyStruct(reflect.TypeOf(productVariables),
+		func(field reflect.StructField, prefix string) (bool, reflect.StructField) {
+			// Filter function, returns true if the field should be in the resulting struct
+			if prefix == "" {
+				// Keep the top level Product_variables field
+				return true, field
+			}
+			_, rest := splitPrefix(prefix)
+			if rest == "" {
+				// Keep the 2nd level field (i.e. Product_variables.Eng)
+				return true, field
+			}
+
+			// Strip off the first 2 levels of the prefix
+			_, prefix = splitPrefix(rest)
+
+			for _, p := range moduleTypeProps {
+				if fieldExistsByNameRecursive(reflect.TypeOf(p).Elem(), prefix, field.Name) {
+					// Keep any fields that exist in one of the property structs
+					return true, field
+				}
+			}
+
+			return false, field
+		})
+	return typ
+}
+
+func splitPrefix(prefix string) (first, rest string) {
+	index := strings.IndexByte(prefix, '.')
+	if index == -1 {
+		return prefix, ""
+	}
+	return prefix[:index], prefix[index+1:]
+}
+
+func fieldExistsByNameRecursive(t reflect.Type, prefix, name string) bool {
+	if t.Kind() != reflect.Struct {
+		panic(fmt.Errorf("fieldExistsByNameRecursive can only be called on a reflect.Struct"))
+	}
+
+	if prefix != "" {
+		split := strings.SplitN(prefix, ".", 2)
+		firstPrefix := split[0]
+		rest := ""
+		if len(split) > 1 {
+			rest = split[1]
+		}
+		f, exists := t.FieldByName(firstPrefix)
+		if !exists {
+			return false
+		}
+		ft := f.Type
+		if ft.Kind() == reflect.Ptr {
+			ft = ft.Elem()
+		}
+		if ft.Kind() != reflect.Struct {
+			panic(fmt.Errorf("field %q in %q is not a struct", firstPrefix, t))
+		}
+		return fieldExistsByNameRecursive(ft, rest, name)
+	} else {
+		_, exists := t.FieldByName(name)
+		return exists
+	}
+}

android/variable_test.go

@@ -16,7 +16,10 @@ package android
 
 import (
 	"reflect"
+	"strconv"
 	"testing"
+
+	"github.com/google/blueprint/proptools"
 )
 
 type printfIntoPropertyTestCase struct {
@@ -122,3 +125,111 @@ func TestPrintfIntoProperty(t *testing.T) {
 		}
 	}
 }
+
+type testProductVariableModule struct {
+	ModuleBase
+}
+
+func (m *testProductVariableModule) GenerateAndroidBuildActions(ctx ModuleContext) {
+}
+
+var testProductVariableProperties = struct {
+	Product_variables struct {
+		Eng struct {
+			Srcs   []string
+			Cflags []string
+		}
+	}
+}{}
+
+func testProductVariableModuleFactoryFactory(props interface{}) func() Module {
+	return func() Module {
+		m := &testProductVariableModule{}
+		clonedProps := proptools.CloneProperties(reflect.ValueOf(props)).Interface()
+		m.AddProperties(clonedProps)
+
+		// Set a default variableProperties, this will be used as the input to the property struct filter
+		// for this test module.
+		m.variableProperties = testProductVariableProperties
+		InitAndroidModule(m)
+		return m
+	}
+}
+
+func TestProductVariables(t *testing.T) {
+	ctx := NewTestContext()
+	// A module type that has a srcs property but not a cflags property.
+	ctx.RegisterModuleType("module1", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct {
+		Srcs []string
+	}{})))
+	// A module type that has a cflags property but not a srcs property.
+	ctx.RegisterModuleType("module2", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct {
+		Cflags []string
+	}{})))
+	// A module type that does not have any properties that match product_variables.
+	ctx.RegisterModuleType("module3", ModuleFactoryAdaptor(testProductVariableModuleFactoryFactory(struct {
+		Foo []string
+	}{})))
+	ctx.PreDepsMutators(func(ctx RegisterMutatorsContext) {
+		ctx.BottomUp("variable", variableMutator).Parallel()
+	})
+
+	// Test that a module can use one product variable even if it doesn't have all the properties
+	// supported by that product variable.
+	bp := `
+		module1 {
+			name: "foo",
+			product_variables: {
+				eng: {
+					srcs: ["foo.c"],
+				},
+			},
+		}
+		module2 {
+			name: "bar",
+			product_variables: {
+				eng: {
+					cflags: ["-DBAR"],
+				},
+			},
+		}
+
+		module3 {
+			name: "baz",
+		}
+	`
+
+	mockFS := map[string][]byte{
+		"Android.bp": []byte(bp),
+	}
+
+	ctx.MockFileSystem(mockFS)
+
+	ctx.Register()
+
+	config := TestConfig(buildDir, nil)
+	config.TestProductVariables.Eng = proptools.BoolPtr(true)
+
+	_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+	FailIfErrored(t, errs)
+	_, errs = ctx.PrepareBuildActions(config)
+	FailIfErrored(t, errs)
+}
+
+func BenchmarkSliceToTypeArray(b *testing.B) {
+	for _, n := range []int{1, 2, 4, 8, 100} {
+		var propStructs []interface{}
+		for i := 0; i < n; i++ {
+			propStructs = append(propStructs, &struct {
+				A *string
+				B string
+			}{})
+
+		}
+		b.Run(strconv.Itoa(n), func(b *testing.B) {
+			for i := 0; i < b.N; i++ {
+				_ = sliceToTypeArray(propStructs)
+			}
+		})
+	}
+}