From 742adc00fe64d46e07011c08bd5b4f060d188752 Mon Sep 17 00:00:00 2001 From: Joe Groocock Date: Mon, 1 Feb 2021 17:56:44 +0000 Subject: [PATCH] lang: Convert StmtRes to engine.Res with types.Into() Replace existing field-mapping code with calls to types.Into() to reflect the mcl data into the Go resource struct with finer granularity and accuracy, and less reliance on the magic reflect.Set() function. One major advantage over reflect.Value.Set() is Into() allows tailoring the data that is set, specifically when coercing mcl struct values into Golang struct values, the fields can be appropriately mapped based on the lang tag on the struct field. With reflect.Value.Set() this was not at all possible as there was a contradiction of logic given the following rules: - mcl struct fields must have lowercase names - Golang struct fields with lowercase names are unexported - Golang reflection does not allow modifying unexported fields Prior to this change, it was impossible to map an mcl inline struct in a resource to the matched Golang counterpart, even if the lang tag was present on the struct field. This can be demonstrated with the following trivial example: test "name" { key => struct{ name => "hello", }, } and the accompanying Golang resource definition: type TestRes struct { traits.Base traits.Edgeable Key struct { Name string `lang:"name"` } `lang:"key"` } Due to the mismatch in field names in the embedded struct, the type unifier failed and refused to match mcl 'name' to Go 'Name' due to the missing mapping logic. Signed-off-by: Joe Groocock --- lang/structs.go | 88 +++++++++---------------------------------------- 1 file changed, 16 insertions(+), 72 deletions(-) diff --git a/lang/structs.go b/lang/structs.go index 38984fff..36609780 100644 --- a/lang/structs.go +++ b/lang/structs.go @@ -604,8 +604,8 @@ func (obj *StmtRes) resource(resName string) (engine.Res, error) { return nil, errwrap.Wrapf(err, "cannot create resource kind `%s` with named `%s`", obj.Kind, resName) } - s := reflect.ValueOf(res).Elem() // pointer to struct, then struct - if k := s.Kind(); k != reflect.Struct { + sv := reflect.ValueOf(res).Elem() // pointer to struct, then struct + if k := sv.Kind(); k != reflect.Struct { panic(fmt.Sprintf("expected struct, got: %s", k)) } @@ -613,7 +613,7 @@ func (obj *StmtRes) resource(resName string) (engine.Res, error) { if err != nil { return nil, err } - ts := reflect.TypeOf(res).Elem() // pointer to struct, then struct + st := reflect.TypeOf(res).Elem() // pointer to struct, then struct // FIXME: we could probably simplify this code... for _, line := range obj.Contents { @@ -638,25 +638,19 @@ func (obj *StmtRes) resource(resName string) (engine.Res, error) { return nil, errwrap.Wrapf(err, "resource field `%s` did not return a type", x.Field) } - fieldValue, err := x.Value.Value() // Value method on Expr - if err != nil { - return nil, err - } - val := fieldValue.Value() // get interface value - name, exists := mapping[x.Field] // lookup recommended field name - if !exists { + if !exists { // this should be caught during unification. return nil, fmt.Errorf("field `%s` does not exist", x.Field) // user made a typo? } - f := s.FieldByName(name) // exported field - if !f.IsValid() || !f.CanSet() { - return nil, fmt.Errorf("field `%s` cannot be set", name) // field is broken? + tf, exists := st.FieldByName(name) // exported field type + if !exists { + return nil, fmt.Errorf("field `%s` type does not exist", x.Field) } - tf, exists := ts.FieldByName(name) // exported field type - if !exists { // illogical because of above check? - return nil, fmt.Errorf("field `%s` type does not exist", x.Field) + f := sv.FieldByName(name) // exported field + if !f.IsValid() || !f.CanSet() { + return nil, fmt.Errorf("field `%s` cannot be set", name) // field is broken? } // is expr type compatible with expected field type? @@ -668,65 +662,15 @@ func (obj *StmtRes) resource(resName string) (engine.Res, error) { return nil, errwrap.Wrapf(err, "resource field `%s` of type `%+v`, cannot take type `%+v", x.Field, t, typ) } - // user `pestle` on #go-nuts irc wrongly insisted that it wasn't - // right to use reflect to do all of this. what is a better way? - - // first iterate through the raw pointers to the underlying type - ttt := tf.Type // ttt is field expected type - tkk := ttt.Kind() - for tkk == reflect.Ptr { - ttt = ttt.Elem() // un-nest one pointer - tkk = ttt.Kind() + fv, err := x.Value.Value() // Value method on Expr + if err != nil { + return nil, err } - // all our int's are src kind == reflect.Int64 in our language! - if obj.data.Debug { - obj.data.Logf("field `%s`: type(%+v), expected(%+v)", x.Field, typ, tkk) + // mutate the struct field f with the mcl data in fv + if err := types.Into(fv, f); err != nil { + return nil, err } - - // overflow check - switch tkk { // match on destination field kind - case reflect.Int, reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8: - ff := reflect.Zero(ttt) // test on a non-ptr equivalent - if ff.OverflowInt(val.(int64)) { // this is valid! - return nil, fmt.Errorf("field `%s` is an `%s`, and value `%d` will overflow it", x.Field, f.Kind(), val) - } - - case reflect.Uint, reflect.Uint64, reflect.Uint32, reflect.Uint16, reflect.Uint8: - ff := reflect.Zero(ttt) - if ff.OverflowUint(uint64(val.(int64))) { // TODO: is this correct? - return nil, fmt.Errorf("field `%s` is an `%s`, and value `%d` will overflow it", x.Field, f.Kind(), val) - } - - case reflect.Float64, reflect.Float32: - ff := reflect.Zero(ttt) - if ff.OverflowFloat(val.(float64)) { - return nil, fmt.Errorf("field `%s` is an `%s`, and value `%d` will overflow it", x.Field, f.Kind(), val) - } - } - - value := reflect.ValueOf(val) // raw value - value = value.Convert(ttt) // now convert our raw value properly - - // finally build a new value to set - tt := tf.Type - kk := tt.Kind() - if obj.data.Debug { - obj.data.Logf("field `%s`: start(%v)->kind(%v)", x.Field, tt, kk) - } - //fmt.Printf("start: %v || %+v\n", tt, kk) - for kk == reflect.Ptr { - tt = tt.Elem() // un-nest one pointer - kk = tt.Kind() - if obj.data.Debug { - obj.data.Logf("field `%s`:\tloop(%v)->kind(%v)", x.Field, tt, kk) - } - // wrap in ptr by one level - valof := reflect.ValueOf(value.Interface()) - value = reflect.New(valof.Type()) - value.Elem().Set(valof) - } - f.Set(value) // set it ! } return res, nil