Support extracting common values from embedded structures

This change also added support for excluding properties from common
value extraction by using a struct tag of `sdk:"keep"` That was needed
to prevent the fields in SdkMemberPropertiesBase from having their
values cleared.

The purpose of this change is to make it easier to share functionality
across sdk member types.

Bug: 142935992
Test: m nothing
Change-Id: Ie5160a8f854056920e411801ca20721eab7c8578

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)
+		}
 	}
 }