diff --git a/parser/ast.go b/parser/ast.go index 57c4948..fb7e516 100644 --- a/parser/ast.go +++ b/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() { diff --git a/parser/parser.go b/parser/parser.go index 6ae5df3..9b6aa18 100644 --- a/parser/parser.go +++ b/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 != ',' { diff --git a/parser/parser_test.go b/parser/parser_test.go index 70151ad..c9d284b 100644 --- a/parser/parser_test.go +++ b/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]) } diff --git a/parser/printer_test.go b/parser/printer_test.go index 6f76b26..077a782 100644 --- a/parser/printer_test.go +++ b/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", + ], + ], } `, }, diff --git a/parser/sort.go b/parser/sort.go index c8bf74f..da594db 100644 --- a/parser/sort.go +++ b/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 + } +} diff --git a/proptools/extend_test.go b/proptools/extend_test.go index d591ce6..0470379 100644 --- a/proptools/extend_test.go +++ b/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 } }{ diff --git a/proptools/proptools.go b/proptools/proptools.go index c44a4a8..2aa6e32 100644 --- a/proptools/proptools.go +++ b/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 +} diff --git a/proptools/unpack.go b/proptools/unpack.go index 344327f..8dc9ca8 100644 --- a/proptools/unpack.go +++ b/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, - propertiesStructs ...interface{}) (map[string]*parser.Property, []error) { +// unpackContext keeps compound names and their values in a map. It is initialized from +// parsed properties. +type unpackContext struct { + propertyMap map[string]*packedProperty + errs []error +} - propertyMap := make(map[string]*packedProperty) - errs := buildPropertyMap("", propertyDefs, propertyMap) - if len(errs) > 0 { - return nil, errs +// UnpackProperties populates the list of runtime values ("property structs") from the parsed properties. +// If a property a.b.c has a value, a field with the matching name in each runtime value is initialized +// from it. See PropertyNameForField for field and property name matching. +// 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 { - propertiesValue := reflect.ValueOf(properties) - if !isStructPtr(propertiesValue.Type()) { + for _, obj := range objects { + valueObject := reflect.ValueOf(obj) + if !isStructPtr(valueObject.Type()) { panic(fmt.Errorf("properties must be *struct, got %s", - propertiesValue.Type())) + valueObject.Type())) } - propertiesValue = propertiesValue.Elem() - - newErrs := unpackStructValue("", propertiesValue, propertyMap) - errs = append(errs, newErrs...) - - if len(errs) >= maxUnpackErrors { - return nil, errs + unpackContext.unpackToStruct("", valueObject.Elem()) + if len(unpackContext.errs) >= maxUnpackErrors { + return nil, unpackContext.errs } } - // Report any properties that didn't have corresponding struct fields as - // errors. + // Gather property map, and collect any unused properties. + // Avoid reporting subproperties of unused properties. result := make(map[string]*parser.Property) - for name, packedProperty := range propertyMap { - result[name] = packedProperty.property - if !packedProperty.unpacked { - err := &UnpackError{ - Err: fmt.Errorf("unrecognized property %q", name), - Pos: packedProperty.property.ColonPos, - } - errs = append(errs, err) + var unusedNames []string + for name, v := range unpackContext.propertyMap { + if v.used { + result[name] = v.property + } else { + unusedNames = append(unusedNames, name) } } - - if len(errs) > 0 { - return nil, errs + if len(unusedNames) == 0 && len(unpackContext.errs) == 0 { + return result, nil } - - return result, nil + return nil, unpackContext.reportUnusedNames(unusedNames) } -func buildPropertyMap(namePrefix string, propertyDefs []*parser.Property, - propertyMap map[string]*packedProperty) (errs []error) { - - for _, propertyDef := range propertyDefs { - name := namePrefix + propertyDef.Name - if first, present := propertyMap[name]; present { - if first.property == propertyDef { - // We've already added this property. +func (ctx *unpackContext) reportUnusedNames(unusedNames []string) []error { + sort.Strings(unusedNames) + var lastReported string + for _, name := range unusedNames { + // if 'foo' has been reported, ignore 'foo\..*' and 'foo\[.*' + if lastReported != "" { + trimmed := strings.TrimPrefix(name, lastReported) + if trimmed != name && (trimmed[0] == '.' || trimmed[0] == '[') { continue } - errs = append(errs, &UnpackError{ - Err: fmt.Errorf("property %q already defined", name), - Pos: propertyDef.ColonPos, - }) - errs = append(errs, &UnpackError{ - Err: fmt.Errorf("<-- previous definition here"), - Pos: first.property.ColonPos, - }) - if len(errs) >= maxUnpackErrors { - return errs + } + ctx.errs = append(ctx.errs, &UnpackError{ + fmt.Errorf("unrecognized property %q", name), + ctx.propertyMap[name].property.ColonPos}) + lastReported = name + } + return ctx.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{ - property: propertyDef, - unpacked: false, + ctx.propertyMap[name] = &packedProperty{property, false} + if structProperty, ok := property.Value.(*parser.Map); ok { + ctx.buildPropertyMap(name, structProperty.Properties) } - // We intentionally do not rescursively add MapValue properties to the - // property map here. Instead we add them when we encounter a struct - // into which they can be unpacked. We do this so that if we never - // encounter such a struct then the "unrecognized property" error will - // be reported only once for the map property and not for each of its - // sub-properties. + // If it is a list, unroll it unless its elements are of primitive type + // (no further mapping will be needed in that case, so we avoid cluttering + // the map). + listExpr, ok := property.Value.Eval().(*parser.List) + if !ok || len(listExpr.Values) == 0 { + 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, - propertyMap map[string]*packedProperty) []error { +func fieldPath(prefix, fieldName string) string { + 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) - errs = append(errs, newErrs...) + ctx.unpackToStruct(namePrefix, fieldValue) 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), - Pos: packedProperty.property.ColonPos, - }) - if len(errs) >= maxUnpackErrors { - return errs + fmt.Errorf("mutated field %s cannot be set in a Blueprint file", propertyName), + property.ColonPos, + }) { + return } continue } - var newErrs []error - if isStruct(fieldValue.Type()) { - newErrs = unpackStruct(propertyName+".", fieldValue, - packedProperty.property, propertyMap) - - errs = append(errs, newErrs...) - if len(errs) >= maxUnpackErrors { - return errs + ctx.unpackToStruct(propertyName, fieldValue) + if len(ctx.errs) >= maxUnpackErrors { + return + } + } else if isSlice(fieldValue.Type()) { + if unpackedValue, ok := ctx.unpackToSlice(propertyName, property, fieldValue.Type()); ok { + ExtendBasicType(fieldValue, unpackedValue, Append) + } + if len(ctx.errs) >= maxUnpackErrors { + return } - continue - } - - // Handle basic types and pointers to basic types - - propertyValue, err := propertyToValue(fieldValue.Type(), packedProperty.property) - if err != nil { - errs = append(errs, err) - if len(errs) >= maxUnpackErrors { - return errs + } else { + unpackedValue, err := propertyToValue(fieldValue.Type(), property) + if err != nil && !ctx.addError(err) { + return } + ExtendBasicType(fieldValue, unpackedValue, Append) } - - ExtendBasicType(fieldValue, propertyValue, Append) } - - return errs } -func propertyToValue(typ reflect.Type, property *parser.Property) (reflect.Value, error) { - var value reflect.Value - - var ptr bool - if typ.Kind() == reflect.Ptr { - typ = typ.Elem() - ptr = true +// unpackSlice creates a value of a given slice type from the property which should be a list +func (ctx *unpackContext) unpackToSlice( + sliceName string, property *parser.Property, sliceType reflect.Type) (reflect.Value, bool) { + propValueAsList, ok := property.Value.Eval().(*parser.List) + if !ok { + ctx.addError(fmt.Errorf("%s: can't assign %s value to list property %q", + 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) -} diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go index 942dbb8..54ec01a 100644 --- a/proptools/unpack_test.go +++ b/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 { - return scanner.Position{ - Offset: offset, - Line: line, - Column: column, +func BenchmarkUnpackProperties(b *testing.B) { + run := func(b *testing.B, props []interface{}, input string) { + b.ReportAllocs() + b.StopTimer() + 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) + }) }