Merge pull request #286 from asmundak/master

Implement list of maps

parser/ast.go

@@ -130,7 +130,7 @@ func ExpressionsAreSame(a Expression, b Expression) (equal bool, err error) {
 	return hackyExpressionsAreSame(a, b)
 }
 
-// TODO(jeffrygaston) once positions are removed from Expression stucts,
+// TODO(jeffrygaston) once positions are removed from Expression structs,
 // remove this function and have callers use reflect.DeepEqual(a, b)
 func hackyExpressionsAreSame(a Expression, b Expression) (equal bool, err error) {
 	if a.Type() != b.Type() {

parser/parser.go

@@ -549,10 +549,6 @@ func (p *parser) parseListValue() *List {
 	var elements []Expression
 	for p.tok != ']' {
 		element := p.parseExpression()
-		if p.eval && element.Type() != StringType {
-			p.errorf("Expected string in list, found %s", element.Type().String())
-			return nil
-		}
 		elements = append(elements, element)
 
 		if p.tok != ',' {

parser/parser_test.go

@@ -193,6 +193,158 @@ var validParseTestCases = []struct {
 		nil,
 	},
 
+	{
+		`
+		foo {
+			list_of_maps: [
+				{
+					var: true,
+					name: "a",
+				},
+				{
+					var: false,
+					name: "b",
+				},
+			],
+		}
+`,
+		[]Definition{
+			&Module{
+				Type:    "foo",
+				TypePos: mkpos(3, 2, 3),
+				Map: Map{
+					LBracePos: mkpos(7, 2, 7),
+					RBracePos: mkpos(127, 13, 3),
+					Properties: []*Property{
+						{
+							Name:     "list_of_maps",
+							NamePos:  mkpos(12, 3, 4),
+							ColonPos: mkpos(24, 3, 16),
+							Value: &List{
+								LBracePos: mkpos(26, 3, 18),
+								RBracePos: mkpos(122, 12, 4),
+								Values: []Expression{
+									&Map{
+										LBracePos: mkpos(32, 4, 5),
+										RBracePos: mkpos(70, 7, 5),
+										Properties: []*Property{
+											{
+												Name:     "var",
+												NamePos:  mkpos(39, 5, 6),
+												ColonPos: mkpos(42, 5, 9),
+												Value: &Bool{
+													LiteralPos: mkpos(44, 5, 11),
+													Value:      true,
+													Token:      "true",
+												},
+											},
+											{
+												Name:     "name",
+												NamePos:  mkpos(55, 6, 6),
+												ColonPos: mkpos(59, 6, 10),
+												Value: &String{
+													LiteralPos: mkpos(61, 6, 12),
+													Value:      "a",
+												},
+											},
+										},
+									},
+									&Map{
+										LBracePos: mkpos(77, 8, 5),
+										RBracePos: mkpos(116, 11, 5),
+										Properties: []*Property{
+											{
+												Name:     "var",
+												NamePos:  mkpos(84, 9, 6),
+												ColonPos: mkpos(87, 9, 9),
+												Value: &Bool{
+													LiteralPos: mkpos(89, 9, 11),
+													Value:      false,
+													Token:      "false",
+												},
+											},
+											{
+												Name:     "name",
+												NamePos:  mkpos(101, 10, 6),
+												ColonPos: mkpos(105, 10, 10),
+												Value: &String{
+													LiteralPos: mkpos(107, 10, 12),
+													Value:      "b",
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		nil,
+	},
+	{
+		`
+		foo {
+			list_of_lists: [
+				[ "a", "b" ],
+				[ "c", "d" ]
+			],
+		}
+`,
+		[]Definition{
+			&Module{
+				Type:    "foo",
+				TypePos: mkpos(3, 2, 3),
+				Map: Map{
+					LBracePos: mkpos(7, 2, 7),
+					RBracePos: mkpos(72, 7, 3),
+					Properties: []*Property{
+						{
+							Name:     "list_of_lists",
+							NamePos:  mkpos(12, 3, 4),
+							ColonPos: mkpos(25, 3, 17),
+							Value: &List{
+								LBracePos: mkpos(27, 3, 19),
+								RBracePos: mkpos(67, 6, 4),
+								Values: []Expression{
+									&List{
+										LBracePos: mkpos(33, 4, 5),
+										RBracePos: mkpos(44, 4, 16),
+										Values: []Expression{
+											&String{
+												LiteralPos: mkpos(35, 4, 7),
+												Value:      "a",
+											},
+											&String{
+												LiteralPos: mkpos(40, 4, 12),
+												Value:      "b",
+											},
+										},
+									},
+									&List{
+										LBracePos: mkpos(51, 5, 5),
+										RBracePos: mkpos(62, 5, 16),
+										Values: []Expression{
+											&String{
+												LiteralPos: mkpos(53, 5, 7),
+												Value:      "c",
+											},
+											&String{
+												LiteralPos: mkpos(58, 5, 12),
+												Value:      "d",
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		nil,
+	},
 	{`
 		foo {
 			stuff: {
@@ -1032,7 +1184,7 @@ func TestParseValidInput(t *testing.T) {
 				for i := range file.Defs {
 					if !reflect.DeepEqual(file.Defs[i], testCase.defs[i]) {
 						t.Errorf("test case: %s", testCase.input)
-						t.Errorf("incorrect defintion %d:", i)
+						t.Errorf("incorrect definition %d:", i)
 						t.Errorf("  expected: %s", testCase.defs[i])
 						t.Errorf("       got: %s", file.Defs[i])
 					}

parser/printer_test.go

@@ -358,6 +358,73 @@ test { // test
 
 // test
 
+}
+`,
+	},
+	{
+		input: `
+// test
+stuff {
+    namespace: "google",
+    string_vars: [
+      {
+          var: "one",
+          values: [ "one_a", "one_b",],
+      },
+      {
+          var: "two",
+          values: [ "two_a", "two_b", ],
+      },
+    ],
+}`,
+		output: `
+// test
+stuff {
+    namespace: "google",
+    string_vars: [
+        {
+            var: "one",
+            values: [
+                "one_a",
+                "one_b",
+            ],
+        },
+        {
+            var: "two",
+            values: [
+                "two_a",
+                "two_b",
+            ],
+        },
+    ],
+}
+`,
+	},
+	{
+		input: `
+// test
+stuff {
+    namespace: "google",
+    list_of_lists: [
+        [ "a", "b" ],
+        [ "c", "d" ],
+    ],
+}
+`,
+		output: `
+// test
+stuff {
+    namespace: "google",
+    list_of_lists: [
+        [
+            "a",
+            "b",
+        ],
+        [
+            "c",
+            "d",
+        ],
+    ],
 }
 `,
 	},

parser/sort.go

@@ -33,6 +33,9 @@ func SortLists(file *File) {
 }
 
 func SortList(file *File, list *List) {
+	if !isListOfPrimitives(list.Values) {
+		return
+	}
 	for i := 0; i < len(list.Values); i++ {
 		// Find a set of values on contiguous lines
 		line := list.Values[i].Pos().Line
@@ -91,6 +94,9 @@ func sortListsInValue(value Expression, file *File) {
 }
 
 func sortSubList(values []Expression, nextPos scanner.Position, file *File) {
+	if !isListOfPrimitives(values) {
+		return
+	}
 	l := make(elemList, len(values))
 	for i, v := range values {
 		s, ok := v.(*String)
@@ -135,6 +141,9 @@ func sortSubList(values []Expression, nextPos scanner.Position, file *File) {
 }
 
 func subListIsSorted(values []Expression) bool {
+	if !isListOfPrimitives(values) {
+		return true
+	}
 	prev := ""
 	for _, v := range values {
 		s, ok := v.(*String)
@@ -184,3 +193,15 @@ func (l commentsByOffset) Less(i, j int) bool {
 func (l commentsByOffset) Swap(i, j int) {
 	l[i], l[j] = l[j], l[i]
 }
+
+func isListOfPrimitives(values []Expression) bool {
+	if len(values) == 0 {
+		return true
+	}
+	switch values[0].Type() {
+	case BoolType, StringType, Int64Type:
+		return true
+	default:
+		return false
+	}
+}

proptools/extend_test.go

@@ -411,6 +411,63 @@ func appendPropertiesTestCases() []appendPropertyTestCase {
 			},
 			order: Replace,
 		},
+		{
+			// Append slice of structs
+			in1: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "foo"}, {F: "bar"},
+				},
+			},
+			in2: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "baz"},
+				},
+			},
+			out: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "foo"}, {F: "bar"}, {F: "baz"},
+				},
+			},
+			order: Append,
+		},
+		{
+			// Prepend slice of structs
+			in1: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "foo"}, {F: "bar"},
+				},
+			},
+			in2: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "baz"},
+				},
+			},
+			out: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "baz"}, {F: "foo"}, {F: "bar"},
+				},
+			},
+			order: Prepend,
+		},
+		{
+			// Replace slice of structs
+			in1: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "foo"}, {F: "bar"},
+				},
+			},
+			in2: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "baz"},
+				},
+			},
+			out: &struct{ S []struct{ F string } }{
+				S: []struct{ F string }{
+					{F: "baz"},
+				},
+			},
+			order: Replace,
+		},
 		{
 			// Append pointer
 			in1: &struct{ S *struct{ S string } }{

proptools/proptools.go

@@ -121,3 +121,7 @@ func isStruct(t reflect.Type) bool {
 func isStructPtr(t reflect.Type) bool {
 	return t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Struct
 }
+
+func isSlice(t reflect.Type) bool {
+	return t.Kind() == reflect.Slice
+}

proptools/unpack.go

@@ -17,6 +17,9 @@ package proptools
 import (
 	"fmt"
 	"reflect"
+	"sort"
+	"strconv"
+	"strings"
 	"text/scanner"
 
 	"github.com/google/blueprint/parser"
@@ -33,103 +36,153 @@ func (e *UnpackError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Pos, e.Err)
 }
 
+// packedProperty helps to track properties usage (`used` will be true)
 type packedProperty struct {
 	property *parser.Property
-	unpacked bool
+	used     bool
 }
 
-func UnpackProperties(propertyDefs []*parser.Property,
+// unpackContext keeps compound names and their values in a map. It is initialized from
-	propertiesStructs ...interface{}) (map[string]*parser.Property, []error) {
+// parsed properties.
+type unpackContext struct {
+	propertyMap map[string]*packedProperty
+	errs        []error
+}
 
-	propertyMap := make(map[string]*packedProperty)
+// UnpackProperties populates the list of runtime values ("property structs") from the parsed properties.
-	errs := buildPropertyMap("", propertyDefs, propertyMap)
+// If a property a.b.c has a value, a field with the matching name in each runtime value is initialized
-	if len(errs) > 0 {
+// from it. See PropertyNameForField for field and property name matching.
-		return nil, errs
+// For instance, if the input contains
+//   { foo: "abc", bar: {x: 1},}
+// and a runtime value being has been declared as
+//   var v struct { Foo string; Bar int }
+// then v.Foo will be set to "abc" and v.Bar will be set to 1
+// (cf. unpack_test.go for further examples)
+//
+// The type of a receiving field has to match the property type, i.e., a bool/int/string field
+// can be set from a property with bool/int/string value, a struct can be set from a map (only the
+// matching fields are set), and an slice can be set from a list.
+// If a field of a runtime value has been already set prior to the UnpackProperties, the new value
+// is appended to it (see somewhat inappropriately named ExtendBasicType).
+// The same property can initialize fields in multiple runtime values. It is an error if any property
+// value was not used to initialize at least one field.
+func UnpackProperties(properties []*parser.Property, objects ...interface{}) (map[string]*parser.Property, []error) {
+	var unpackContext unpackContext
+	unpackContext.propertyMap = make(map[string]*packedProperty)
+	if !unpackContext.buildPropertyMap("", properties) {
+		return nil, unpackContext.errs
 	}
 
-	for _, properties := range propertiesStructs {
+	for _, obj := range objects {
-		propertiesValue := reflect.ValueOf(properties)
+		valueObject := reflect.ValueOf(obj)
-		if !isStructPtr(propertiesValue.Type()) {
+		if !isStructPtr(valueObject.Type()) {
 			panic(fmt.Errorf("properties must be *struct, got %s",
-				propertiesValue.Type()))
+				valueObject.Type()))
 		}
-		propertiesValue = propertiesValue.Elem()
+		unpackContext.unpackToStruct("", valueObject.Elem())
-
+		if len(unpackContext.errs) >= maxUnpackErrors {
-		newErrs := unpackStructValue("", propertiesValue, propertyMap)
+			return nil, unpackContext.errs
-		errs = append(errs, newErrs...)
-
-		if len(errs) >= maxUnpackErrors {
-			return nil, errs
 		}
 	}
 
-	// Report any properties that didn't have corresponding struct fields as
+	// Gather property map, and collect any unused properties.
-	// errors.
+	// Avoid reporting subproperties of unused properties.
 	result := make(map[string]*parser.Property)
-	for name, packedProperty := range propertyMap {
+	var unusedNames []string
-		result[name] = packedProperty.property
+	for name, v := range unpackContext.propertyMap {
-		if !packedProperty.unpacked {
+		if v.used {
-			err := &UnpackError{
+			result[name] = v.property
-				Err: fmt.Errorf("unrecognized property %q", name),
+		} else {
-				Pos: packedProperty.property.ColonPos,
+			unusedNames = append(unusedNames, name)
-			}
-			errs = append(errs, err)
 		}
 	}
-
+	if len(unusedNames) == 0 && len(unpackContext.errs) == 0 {
-	if len(errs) > 0 {
+		return result, nil
-		return nil, errs
 	}
-
+	return nil, unpackContext.reportUnusedNames(unusedNames)
-	return result, nil
 }
 
-func buildPropertyMap(namePrefix string, propertyDefs []*parser.Property,
+func (ctx *unpackContext) reportUnusedNames(unusedNames []string) []error {
-	propertyMap map[string]*packedProperty) (errs []error) {
+	sort.Strings(unusedNames)
-
+	var lastReported string
-	for _, propertyDef := range propertyDefs {
+	for _, name := range unusedNames {
-		name := namePrefix + propertyDef.Name
+		// if 'foo' has been reported, ignore 'foo\..*' and 'foo\[.*'
-		if first, present := propertyMap[name]; present {
+		if lastReported != "" {
-			if first.property == propertyDef {
+			trimmed := strings.TrimPrefix(name, lastReported)
-				// We've already added this property.
+			if trimmed != name && (trimmed[0] == '.' || trimmed[0] == '[') {
 				continue
 			}
-			errs = append(errs, &UnpackError{
+		}
-				Err: fmt.Errorf("property %q already defined", name),
+		ctx.errs = append(ctx.errs, &UnpackError{
-				Pos: propertyDef.ColonPos,
+			fmt.Errorf("unrecognized property %q", name),
-			})
+			ctx.propertyMap[name].property.ColonPos})
-			errs = append(errs, &UnpackError{
+		lastReported = name
-				Err: fmt.Errorf("<-- previous definition here"),
+	}
-				Pos: first.property.ColonPos,
+	return ctx.errs
-			})
+}
-			if len(errs) >= maxUnpackErrors {
+
-				return errs
+func (ctx *unpackContext) buildPropertyMap(prefix string, properties []*parser.Property) bool {
+	nOldErrors := len(ctx.errs)
+	for _, property := range properties {
+		name := fieldPath(prefix, property.Name)
+		if first, present := ctx.propertyMap[name]; present {
+			ctx.addError(
+				&UnpackError{fmt.Errorf("property %q already defined", name), property.ColonPos})
+			if ctx.addError(
+				&UnpackError{fmt.Errorf("<-- previous definition here"), first.property.ColonPos}) {
+				return false
 			}
 			continue
 		}
 
-		propertyMap[name] = &packedProperty{
+		ctx.propertyMap[name] = &packedProperty{property, false}
-			property: propertyDef,
+		if structProperty, ok := property.Value.(*parser.Map); ok {
-			unpacked: false,
+			ctx.buildPropertyMap(name, structProperty.Properties)
 		}
 
-		// We intentionally do not rescursively add MapValue properties to the
+		// If it is a list, unroll it unless its elements are of primitive type
-		// property map here.  Instead we add them when we encounter a struct
+		// (no further mapping will be needed in that case, so we avoid cluttering
-		// into which they can be unpacked.  We do this so that if we never
+		// the map).
-		// encounter such a struct then the "unrecognized property" error will
+		listExpr, ok := property.Value.Eval().(*parser.List)
-		// be reported only once for the map property and not for each of its
+		if !ok || len(listExpr.Values) == 0 {
-		// sub-properties.
+			continue
+		}
+		if t := listExpr.Values[0].Eval().Type(); t == parser.StringType || t == parser.Int64Type || t == parser.BoolType {
+			continue
+		}
+
+		itemProperties := make([]*parser.Property, len(listExpr.Values), len(listExpr.Values))
+		for i, expr := range listExpr.Values {
+			itemProperties[i] = &parser.Property{
+				Name:     property.Name + "[" + strconv.Itoa(i) + "]",
+				NamePos:  property.NamePos,
+				ColonPos: property.ColonPos,
+				Value:    expr,
+			}
+		}
+		if !ctx.buildPropertyMap(prefix, itemProperties) {
+			return false
+		}
 	}
 
-	return
+	return len(ctx.errs) == nOldErrors
 }
 
-func unpackStructValue(namePrefix string, structValue reflect.Value,
+func fieldPath(prefix, fieldName string) string {
-	propertyMap map[string]*packedProperty) []error {
+	if prefix == "" {
+		return fieldName
+	}
+	return prefix + "." + fieldName
+}
 
+func (ctx *unpackContext) addError(e error) bool {
+	ctx.errs = append(ctx.errs, e)
+	return len(ctx.errs) < maxUnpackErrors
+}
+
+func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect.Value) {
 	structType := structValue.Type()
 
-	var errs []error
 	for i := 0; i < structValue.NumField(); i++ {
 		fieldValue := structValue.Field(i)
 		field := structType.Field(i)
@@ -148,14 +201,14 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 			continue
 		}
 
-		propertyName := namePrefix + PropertyNameForField(field.Name)
+		propertyName := fieldPath(namePrefix, PropertyNameForField(field.Name))
 
 		if !fieldValue.CanSet() {
 			panic(fmt.Errorf("field %s is not settable", propertyName))
 		}
 
 		// Get the property value if it was specified.
-		packedProperty, propertyIsSet := propertyMap[propertyName]
+		packedProperty, propertyIsSet := ctx.propertyMap[propertyName]
 
 		origFieldValue := fieldValue
 
@@ -164,15 +217,8 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 		// TODO(ccross): we don't validate types inside nil struct pointers
 		// Move type validation to a function that runs on each factory once
 		switch kind := fieldValue.Kind(); kind {
-		case reflect.Bool, reflect.String, reflect.Struct:
+		case reflect.Bool, reflect.String, reflect.Struct, reflect.Slice:
 			// Do nothing
-		case reflect.Slice:
-			elemType := field.Type.Elem()
-			if elemType.Kind() != reflect.String {
-				if !HasTag(field, "blueprint", "mutated") {
-					panic(fmt.Errorf("field %s is a non-string slice", propertyName))
-				}
-			}
 		case reflect.Interface:
 			if fieldValue.IsNil() {
 				panic(fmt.Errorf("field %s contains a nil interface", propertyName))
@@ -210,8 +256,7 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 		}
 
 		if field.Anonymous && isStruct(fieldValue.Type()) {
-			newErrs := unpackStructValue(namePrefix, fieldValue, propertyMap)
+			ctx.unpackToStruct(namePrefix, fieldValue)
-			errs = append(errs, newErrs...)
 			continue
 		}
 
@@ -220,60 +265,125 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
 			continue
 		}
 
-		packedProperty.unpacked = true
+		packedProperty.used = true
+		property := packedProperty.property
 
 		if HasTag(field, "blueprint", "mutated") {
-			errs = append(errs,
+			if !ctx.addError(
 				&UnpackError{
-					Err: fmt.Errorf("mutated field %s cannot be set in a Blueprint file", propertyName),
+					fmt.Errorf("mutated field %s cannot be set in a Blueprint file", propertyName),
-					Pos: packedProperty.property.ColonPos,
+					property.ColonPos,
-				})
+				}) {
-			if len(errs) >= maxUnpackErrors {
+				return
-				return errs
 			}
 			continue
 		}
 
-		var newErrs []error
-
 		if isStruct(fieldValue.Type()) {
-			newErrs = unpackStruct(propertyName+".", fieldValue,
+			ctx.unpackToStruct(propertyName, fieldValue)
-				packedProperty.property, propertyMap)
+			if len(ctx.errs) >= maxUnpackErrors {
-
+				return
-			errs = append(errs, newErrs...)
+			}
-			if len(errs) >= maxUnpackErrors {
+		} else if isSlice(fieldValue.Type()) {
-				return errs
+			if unpackedValue, ok := ctx.unpackToSlice(propertyName, property, fieldValue.Type()); ok {
+				ExtendBasicType(fieldValue, unpackedValue, Append)
+			}
+			if len(ctx.errs) >= maxUnpackErrors {
+				return
 			}
 
-			continue
+		} else {
-		}
+			unpackedValue, err := propertyToValue(fieldValue.Type(), property)
-
+			if err != nil && !ctx.addError(err) {
-		// Handle basic types and pointers to basic types
+				return
-
-		propertyValue, err := propertyToValue(fieldValue.Type(), packedProperty.property)
-		if err != nil {
-			errs = append(errs, err)
-			if len(errs) >= maxUnpackErrors {
-				return errs
 			}
+			ExtendBasicType(fieldValue, unpackedValue, Append)
 		}
-
-		ExtendBasicType(fieldValue, propertyValue, Append)
 	}
-
-	return errs
 }
 
-func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value, error) {
+// unpackSlice creates a value of a given slice type from the property which should be a list
-	var value reflect.Value
+func (ctx *unpackContext) unpackToSlice(
-
+	sliceName string, property *parser.Property, sliceType reflect.Type) (reflect.Value, bool) {
-	var ptr bool
+	propValueAsList, ok := property.Value.Eval().(*parser.List)
-	if typ.Kind() == reflect.Ptr {
+	if !ok {
-		typ = typ.Elem()
+		ctx.addError(fmt.Errorf("%s: can't assign %s value to list property %q",
-		ptr = true
+			property.Value.Pos(), property.Value.Type(), property.Name))
+		return reflect.MakeSlice(sliceType, 0, 0), false
+	}
+	exprs := propValueAsList.Values
+	value := reflect.MakeSlice(sliceType, 0, len(exprs))
+	if len(exprs) == 0 {
+		return value, true
 	}
 
-	switch kind := typ.Kind(); kind {
+	// The function to construct an item value depends on the type of list elements.
+	var getItemFunc func(*parser.Property, reflect.Type) (reflect.Value, bool)
+	switch exprs[0].Type() {
+	case parser.BoolType, parser.StringType, parser.Int64Type:
+		getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) {
+			value, err := propertyToValue(t, property)
+			if err != nil {
+				ctx.addError(err)
+				return value, false
+			}
+			return value, true
+		}
+	case parser.ListType:
+		getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) {
+			return ctx.unpackToSlice(property.Name, property, t)
+		}
+	case parser.MapType:
+		getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) {
+			itemValue := reflect.New(t).Elem()
+			ctx.unpackToStruct(property.Name, itemValue)
+			return itemValue, true
+		}
+	case parser.NotEvaluatedType:
+		getItemFunc = func(property *parser.Property, t reflect.Type) (reflect.Value, bool) {
+			return reflect.New(t), false
+		}
+	default:
+		panic(fmt.Errorf("bizarre property expression type: %v", exprs[0].Type()))
+	}
+
+	itemProperty := &parser.Property{NamePos: property.NamePos, ColonPos: property.ColonPos}
+	elemType := sliceType.Elem()
+	isPtr := elemType.Kind() == reflect.Ptr
+
+	for i, expr := range exprs {
+		itemProperty.Name = sliceName + "[" + strconv.Itoa(i) + "]"
+		itemProperty.Value = expr
+		if packedProperty, ok := ctx.propertyMap[itemProperty.Name]; ok {
+			packedProperty.used = true
+		}
+		if isPtr {
+			if itemValue, ok := getItemFunc(itemProperty, elemType.Elem()); ok {
+				ptrValue := reflect.New(itemValue.Type())
+				ptrValue.Elem().Set(itemValue)
+				value = reflect.Append(value, ptrValue)
+			}
+		} else {
+			if itemValue, ok := getItemFunc(itemProperty, elemType); ok {
+				value = reflect.Append(value, itemValue)
+			}
+		}
+	}
+	return value, true
+}
+
+// propertyToValue creates a value of a given value type from the property.
+func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value, error) {
+	var value reflect.Value
+	var baseType reflect.Type
+	isPtr := typ.Kind() == reflect.Ptr
+	if isPtr {
+		baseType = typ.Elem()
+	} else {
+		baseType = typ
+	}
+
+	switch kind := baseType.Kind(); kind {
 	case reflect.Bool:
 		b, ok := property.Value.Eval().(*parser.Bool)
 		if !ok {
@@ -298,53 +408,16 @@ func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value
 		}
 		value = reflect.ValueOf(s.Value)
 
-	case reflect.Slice:
-		l, ok := property.Value.Eval().(*parser.List)
-		if !ok {
-			return value, fmt.Errorf("%s: can't assign %s value to list property %q",
-				property.Value.Pos(), property.Value.Type(), property.Name)
-		}
-
-		list := make([]string, len(l.Values))
-		for i, value := range l.Values {
-			s, ok := value.Eval().(*parser.String)
-			if !ok {
-				// The parser should not produce this.
-				panic(fmt.Errorf("non-string value %q found in list", value))
-			}
-			list[i] = s.Value
-		}
-
-		value = reflect.ValueOf(list)
-
 	default:
-		panic(fmt.Errorf("unexpected kind %s", kind))
+		return value, &UnpackError{
+			fmt.Errorf("cannot assign %s value %s to %s property %s", property.Value.Type(), property.Value, kind, typ),
+			property.NamePos}
 	}
 
-	if ptr {
+	if isPtr {
 		ptrValue := reflect.New(value.Type())
 		ptrValue.Elem().Set(value)
-		value = ptrValue
+		return ptrValue, nil
 	}
-
 	return value, nil
 }
-
-func unpackStruct(namePrefix string, structValue reflect.Value,
-	property *parser.Property, propertyMap map[string]*packedProperty) []error {
-
-	m, ok := property.Value.Eval().(*parser.Map)
-	if !ok {
-		return []error{
-			fmt.Errorf("%s: can't assign %s value to map property %q",
-				property.Value.Pos(), property.Value.Type(), property.Name),
-		}
-	}
-
-	errs := buildPropertyMap(namePrefix, m.Properties, propertyMap)
-	if len(errs) > 0 {
-		return errs
-	}
-
-	return unpackStructValue(namePrefix, structValue, propertyMap)
-}

proptools/unpack_test.go

@@ -17,8 +17,8 @@ package proptools
 import (
 	"bytes"
 	"reflect"
+
 	"testing"
-	"text/scanner"
 
 	"github.com/google/blueprint/parser"
 )
@@ -218,6 +218,144 @@ var validUnpackTestCases = []struct {
 		},
 	},
 
+	// List of maps
+	{
+		input: `
+			m {
+				mapslist: [
+					{
+						foo: "abc",
+						bar: true,
+					},
+					{
+						foo: "def",
+						bar: false,
+					}
+				],
+			}
+		`,
+		output: []interface{}{
+			&struct {
+				Mapslist []struct {
+					Foo string
+					Bar bool
+				}
+			}{
+				Mapslist: []struct {
+					Foo string
+					Bar bool
+				}{
+					{Foo: "abc", Bar: true},
+					{Foo: "def", Bar: false},
+				},
+			},
+		},
+	},
+
+	// List of pointers to structs
+	{
+		input: `
+			m {
+				mapslist: [
+					{
+						foo: "abc",
+						bar: true,
+					},
+					{
+						foo: "def",
+						bar: false,
+					}
+				],
+			}
+		`,
+		output: []interface{}{
+			&struct {
+				Mapslist []*struct {
+					Foo string
+					Bar bool
+				}
+			}{
+				Mapslist: []*struct {
+					Foo string
+					Bar bool
+				}{
+					{Foo: "abc", Bar: true},
+					{Foo: "def", Bar: false},
+				},
+			},
+		},
+	},
+
+	// List of lists
+	{
+		input: `
+			m {
+				listoflists: [
+					["abc",],
+					["def",],
+				],
+			}
+		`,
+		output: []interface{}{
+			&struct {
+				Listoflists [][]string
+			}{
+				Listoflists: [][]string{
+					[]string{"abc"},
+					[]string{"def"},
+				},
+			},
+		},
+	},
+
+	// Multilevel
+	{
+		input: `
+			m {
+				name: "mymodule",
+				flag: true,
+				settings: ["foo1", "foo2", "foo3",],
+				perarch: {
+					arm: "32",
+					arm64: "64",
+				},
+				configvars: [
+					{ var: "var1", values: ["1.1", "1.2", ], },
+					{ var: "var2", values: ["2.1", ], },
+				],
+            }
+        `,
+		output: []interface{}{
+			&struct {
+				Name     string
+				Flag     bool
+				Settings []string
+				Perarch  *struct {
+					Arm   string
+					Arm64 string
+				}
+				Configvars []struct {
+					Var    string
+					Values []string
+				}
+			}{
+				Name:     "mymodule",
+				Flag:     true,
+				Settings: []string{"foo1", "foo2", "foo3"},
+				Perarch: &struct {
+					Arm   string
+					Arm64 string
+				}{Arm: "32", Arm64: "64"},
+				Configvars: []struct {
+					Var    string
+					Values []string
+				}{
+					{Var: "var1", Values: []string{"1.1", "1.2"}},
+					{Var: "var2", Values: []string{"2.1"}},
+				},
+			},
+		},
+	},
 	// Anonymous struct
 	{
 		input: `
@@ -585,10 +723,185 @@ func TestUnpackProperties(t *testing.T) {
 	}
 }
 
-func mkpos(offset, line, column int) scanner.Position {
+func BenchmarkUnpackProperties(b *testing.B) {
-	return scanner.Position{
+	run := func(b *testing.B, props []interface{}, input string) {
-		Offset: offset,
+		b.ReportAllocs()
-		Line:   line,
+		b.StopTimer()
-		Column: column,
+		r := bytes.NewBufferString(input)
+		file, errs := parser.ParseAndEval("", r, parser.NewScope(nil))
+		if len(errs) != 0 {
+			b.Errorf("test case: %s", input)
+			b.Errorf("unexpected parse errors:")
+			for _, err := range errs {
+				b.Errorf("  %s", err)
+			}
+			b.FailNow()
+		}
+
+		for i := 0; i < b.N; i++ {
+			for _, def := range file.Defs {
+				module, ok := def.(*parser.Module)
+				if !ok {
+					continue
+				}
+
+				var output []interface{}
+				for _, p := range props {
+					output = append(output, CloneProperties(reflect.ValueOf(p)).Interface())
+				}
+
+				b.StartTimer()
+				_, errs = UnpackProperties(module.Properties, output...)
+				b.StopTimer()
+				if len(errs) > 0 {
+					b.Errorf("unexpected unpack errors:")
+					for _, err := range errs {
+						b.Errorf("  %s", err)
+					}
+				}
+			}
+		}
 	}
+
+	b.Run("basic", func(b *testing.B) {
+		props := []interface{}{
+			&struct {
+				Nested struct {
+					S string
+				}
+			}{},
+		}
+		bp := `
+			m {
+				nested: {
+					s: "abc",
+				},
+			}
+		`
+		run(b, props, bp)
+	})
+
+	b.Run("interface", func(b *testing.B) {
+		props := []interface{}{
+			&struct {
+				Nested interface{}
+			}{
+				Nested: (*struct {
+					S string
+				})(nil),
+			},
+		}
+		bp := `
+			m {
+				nested: {
+					s: "abc",
+				},
+			}
+		`
+		run(b, props, bp)
+	})
+
+	b.Run("many", func(b *testing.B) {
+		props := []interface{}{
+			&struct {
+				A *string
+				B *string
+				C *string
+				D *string
+				E *string
+				F *string
+				G *string
+				H *string
+				I *string
+				J *string
+			}{},
+		}
+		bp := `
+			m {
+				a: "a",
+				b: "b",
+				c: "c",
+				d: "d",
+				e: "e",
+				f: "f",
+				g: "g",
+				h: "h",
+				i: "i",
+				j: "j",
+			}
+		`
+		run(b, props, bp)
+	})
+
+	b.Run("deep", func(b *testing.B) {
+		props := []interface{}{
+			&struct {
+				Nested struct {
+					Nested struct {
+						Nested struct {
+							Nested struct {
+								Nested struct {
+									Nested struct {
+										Nested struct {
+											Nested struct {
+												Nested struct {
+													Nested struct {
+														S string
+													}
+												}
+											}
+										}
+									}
+								}
+							}
+						}
+					}
+				}
+			}{},
+		}
+		bp := `
+			m {
+				nested: { nested: { nested: { nested: { nested: {
+					nested: { nested: { nested: { nested: { nested: {
+						s: "abc",
+					}, }, }, }, },
+				}, }, }, }, },
+			}
+		`
+		run(b, props, bp)
+	})
+
+	b.Run("mix", func(b *testing.B) {
+		props := []interface{}{
+			&struct {
+				Name     string
+				Flag     bool
+				Settings []string
+				Perarch  *struct {
+					Arm   string
+					Arm64 string
+				}
+				Configvars []struct {
+					Name   string
+					Values []string
+				}
+			}{},
+		}
+		bp := `
+			m {
+				name: "mymodule",
+				flag: true,
+				settings: ["foo1", "foo2", "foo3",],
+				perarch: {
+					arm: "32",
+					arm64: "64",
+				},
+				configvars: [
+					{ name: "var1", values: ["var1:1", "var1:2", ], },
+					{ name: "var2", values: ["var2:1", "var2:2", ], },
+				],
+            }
+        `
+		run(b, props, bp)
+	})
 }