Merge "Support extracting common values from embedded structures"

android/sdk.go

@@ -325,7 +325,8 @@ type SdkMemberType interface {
 	//
 	// * The variant property structs are analysed to find exported (capitalized) fields which
 	//   have common values. Those fields are cleared and the common value added to the common
-	//   properties.
+	//   properties. A field annotated with a tag of `sdk:"keep"` will be treated as if it
+	//   was not capitalized, i.e. not optimized for common values.
 	//
 	// * The sdk module type populates the BpModule structure, creating the arch specific
 	//   structure and calls AddToPropertySet(...) on the properties struct to add the member
@@ -452,13 +453,13 @@ func RegisterSdkMemberType(memberType SdkMemberType) {
 // are not affected by the optimization to extract common values.
 type SdkMemberPropertiesBase struct {
 	// The setting to use for the compile_multilib property.
-	Compile_multilib string
+	Compile_multilib string `sdk:"keep"`
 
 	// The number of unique os types supported by the member variants.
-	Os_count int
+	Os_count int `sdk:"keep"`
 
 	// The os type for which these properties refer.
-	Os OsType
+	Os OsType `sdk:"keep"`
 }
 
 // The os prefix to use for any file paths in the sdk.

sdk/sdk_test.go

@@ -16,6 +16,8 @@ package sdk
 
 import (
 	"testing"
+
+	"github.com/google/blueprint/proptools"
 )
 
 // Needed in an _test.go file in this package to ensure tests run correctly, particularly in IDE.
@@ -222,3 +224,106 @@ func TestSDkInstall(t *testing.T) {
 		checkAllOtherCopyRules(`.intermediates/mysdk/common_os/mysdk-current.zip -> mysdk-current.zip`),
 	)
 }
+
+type EmbeddedPropertiesStruct struct {
+	S_Embedded_Common    string
+	S_Embedded_Different string
+}
+
+type testPropertiesStruct struct {
+	private     string
+	Public_Kept string `sdk:"keep"`
+	S_Common    string
+	S_Different string
+	A_Common    []string
+	A_Different []string
+	F_Common    *bool
+	F_Different *bool
+	EmbeddedPropertiesStruct
+}
+
+func TestCommonValueOptimization(t *testing.T) {
+	common := &testPropertiesStruct{}
+	structs := []*testPropertiesStruct{
+		&testPropertiesStruct{
+			private:     "common",
+			Public_Kept: "common",
+			S_Common:    "common",
+			S_Different: "upper",
+			A_Common:    []string{"first", "second"},
+			A_Different: []string{"alpha", "beta"},
+			F_Common:    proptools.BoolPtr(false),
+			F_Different: proptools.BoolPtr(false),
+			EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{
+				S_Embedded_Common:    "embedded_common",
+				S_Embedded_Different: "embedded_upper",
+			},
+		},
+		&testPropertiesStruct{
+			private:     "common",
+			Public_Kept: "common",
+			S_Common:    "common",
+			S_Different: "lower",
+			A_Common:    []string{"first", "second"},
+			A_Different: []string{"alpha", "delta"},
+			F_Common:    proptools.BoolPtr(false),
+			F_Different: proptools.BoolPtr(true),
+			EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{
+				S_Embedded_Common:    "embedded_common",
+				S_Embedded_Different: "embedded_lower",
+			},
+		},
+	}
+
+	extractor := newCommonValueExtractor(common)
+	extractor.extractCommonProperties(common, structs)
+
+	h := TestHelper{t}
+	h.AssertDeepEquals("common properties not correct", common,
+		&testPropertiesStruct{
+			private:     "",
+			Public_Kept: "",
+			S_Common:    "common",
+			S_Different: "",
+			A_Common:    []string{"first", "second"},
+			A_Different: []string(nil),
+			F_Common:    proptools.BoolPtr(false),
+			F_Different: nil,
+			EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{
+				S_Embedded_Common:    "embedded_common",
+				S_Embedded_Different: "",
+			},
+		})
+
+	h.AssertDeepEquals("updated properties[0] not correct", structs[0],
+		&testPropertiesStruct{
+			private:     "common",
+			Public_Kept: "common",
+			S_Common:    "",
+			S_Different: "upper",
+			A_Common:    nil,
+			A_Different: []string{"alpha", "beta"},
+			F_Common:    nil,
+			F_Different: proptools.BoolPtr(false),
+			EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{
+				S_Embedded_Common:    "",
+				S_Embedded_Different: "embedded_upper",
+			},
+		})
+
+	h.AssertDeepEquals("updated properties[1] not correct", structs[1],
+		&testPropertiesStruct{
+			private:     "common",
+			Public_Kept: "common",
+			S_Common:    "",
+			S_Different: "lower",
+			A_Common:    nil,
+			A_Different: []string{"alpha", "delta"},
+			F_Common:    nil,
+			F_Different: proptools.BoolPtr(true),
+			EmbeddedPropertiesStruct: EmbeddedPropertiesStruct{
+				S_Embedded_Common:    "",
+				S_Embedded_Different: "embedded_lower",
+			},
+		})
+}

sdk/testing.go

@@ -19,6 +19,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -176,6 +177,13 @@ func (h *TestHelper) AssertTrimmedStringEquals(message string, expected string,
 	h.AssertStringEquals(message, strings.TrimSpace(expected), strings.TrimSpace(actual))
 }
 
+func (h *TestHelper) AssertDeepEquals(message string, expected interface{}, actual interface{}) {
+	h.t.Helper()
+	if !reflect.DeepEqual(actual, expected) {
+		h.t.Errorf("%s: expected %#v, actual %#v", message, expected, actual)
+	}
+}
+
 // Encapsulates result of processing an SDK definition. Provides support for
 // checking the state of the build structures.
 type testSdkResult struct {

sdk/update.go

@@ -951,7 +951,8 @@ func (s *sdk) getPossibleOsTypes() []android.OsType {
 	return osTypes
 }
 
-// Given a struct value, access a field within that struct.
+// Given a struct value, access a field within that struct (or one of its embedded
+// structs).
 type fieldAccessorFunc func(structValue reflect.Value) reflect.Value
 
 // Supports extracting common values from a number of instances of a properties
@@ -969,13 +970,18 @@ type commonValueExtractor struct {
 func newCommonValueExtractor(propertiesStruct interface{}) *commonValueExtractor {
 	structType := getStructValue(reflect.ValueOf(propertiesStruct)).Type()
 	extractor := &commonValueExtractor{}
-	extractor.gatherFields(structType)
+	extractor.gatherFields(structType, nil)
 	return extractor
 }
 
 // Gather the fields from the supplied structure type from which common values will
 // be extracted.
-func (e *commonValueExtractor) gatherFields(structType reflect.Type) {
+//
+// This is recursive function. If it encounters an embedded field (no field name)
+// that is a struct then it will recurse into that struct passing in the accessor
+// for the field. That will then be used in the accessors for the fields in the
+// embedded struct.
+func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingStructAccessor fieldAccessorFunc) {
 	for f := 0; f < structType.NumField(); f++ {
 		field := structType.Field(f)
 		if field.PkgPath != "" {
@@ -983,14 +989,20 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type) {
 			continue
 		}
 
-		// Ignore embedded structures.
-		if field.Type.Kind() == reflect.Struct && field.Anonymous {
+		// Ignore fields whose value should be kept.
+		if proptools.HasTag(field, "sdk", "keep") {
 			continue
 		}
 
 		// Save a copy of the field index for use in the function.
 		fieldIndex := f
 		fieldGetter := func(value reflect.Value) reflect.Value {
+			if containingStructAccessor != nil {
+				// This is an embedded structure so first access the field for the embedded
+				// structure.
+				value = containingStructAccessor(value)
+			}
+
 			// Skip through interface and pointer values to find the structure.
 			value = getStructValue(value)
 
@@ -998,7 +1010,12 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type) {
 			return value.Field(fieldIndex)
 		}
 
-		e.fieldGetters = append(e.fieldGetters, fieldGetter)
+		if field.Type.Kind() == reflect.Struct && field.Anonymous {
+			// Gather fields from the embedded structure.
+			e.gatherFields(field.Type, fieldGetter)
+		} else {
+			e.fieldGetters = append(e.fieldGetters, fieldGetter)
+		}
 	}
 }