From e4bad9c1799aa6caf0f1908f0ba4adaaeb9ce6d6 Mon Sep 17 00:00:00 2001 From: deelawn Date: Mon, 15 Jan 2024 22:17:59 +0000 Subject: [PATCH] style: Increased append readability (#1350) I've been looking at append code a lot recently and the current variable naming makes it difficult for me to quickly understand what I'm looking at; these changes help.
Contributors' checklist... - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
--- gnovm/pkg/gnolang/uverse.go | 352 ++++++++++++++++++------------------ 1 file changed, 174 insertions(+), 178 deletions(-) diff --git a/gnovm/pkg/gnolang/uverse.go b/gnovm/pkg/gnolang/uverse.go index 041f1557e61..a0e9913eaad 100644 --- a/gnovm/pkg/gnolang/uverse.go +++ b/gnovm/pkg/gnolang/uverse.go @@ -147,32 +147,32 @@ func UverseNode() *PackageNode { // As a special case, if arg1 is a string type, first convert it into // a data slice type. if arg1.TV.T != nil && arg1.TV.T.Kind() == StringKind { - arg1s := arg1.TV.GetString() + arg1String := arg1.TV.GetString() // NOTE: this hack works because // arg1 PointerValue is not a pointer, // so the modification here is only local. - av := m.Alloc.NewDataArray(len(arg1s)) - copy(av.Data, []byte(arg1s)) + newArrayValue := m.Alloc.NewDataArray(len(arg1String)) + copy(newArrayValue.Data, []byte(arg1String)) arg1.TV = &TypedValue{ T: m.Alloc.NewType(&SliceType{ // TODO: reuse Elt: Uint8Type, Vrd: true, }), - V: m.Alloc.NewSlice(av, 0, len(arg1s), len(arg1s)), // TODO: pool? + V: m.Alloc.NewSlice(newArrayValue, 0, len(arg1String), len(arg1String)), // TODO: pool? } } - xt := arg0.TV.T - argt := arg1.TV.T - switch xv := arg0.TV.V.(type) { + arg0Type := arg0.TV.T + arg1Type := arg1.TV.T + switch arg0Value := arg0.TV.V.(type) { // ---------------------------------------------------------------- // append(nil, ???) case nil: - switch args := arg1.TV.V.(type) { + switch arg1Value := arg1.TV.V.(type) { // ------------------------------------------------------------ // append(nil, nil) case nil: // no change m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: nil, }) return @@ -180,42 +180,44 @@ func UverseNode() *PackageNode { // ------------------------------------------------------------ // append(nil, *SliceValue) case *SliceValue: - argsl := args.Length - argso := args.Offset - argsb := args.GetBase(m.Store) - if argsl == 0 { // no change + arg1Length := arg1Value.Length + arg1Offset := arg1Value.Offset + arg1Base := arg1Value.GetBase(m.Store) + arg1EndIndex := arg1Offset + arg1Length + + if arg1Length == 0 { // no change m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: nil, }) return - } else if xt.Elem().Kind() == Uint8Kind { + } else if arg0Type.Elem().Kind() == Uint8Kind { // append(nil, *SliceValue) new data bytes --- - data := make([]byte, argsl) - if argsb.Data == nil { + data := make([]byte, arg1Length) + if arg1Base.Data == nil { copyListToData( - data[:argsl], - argsb.List[argso:argso+argsl]) + data[:arg1Length], + arg1Base.List[arg1Offset:arg1EndIndex]) } else { copy( - data[:argsl], - argsb.Data[argso:argso+argsl]) + data[:arg1Length], + arg1Base.Data[arg1Offset:arg1EndIndex]) } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromData(data), }) return } else { // append(nil, *SliceValue) new list --------- - list := make([]TypedValue, argsl) - if 0 < argsl { - for i := 0; i < argsl; i++ { - list[i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) + list := make([]TypedValue, arg1Length) + if 0 < arg1Length { + for i := 0; i < arg1Length; i++ { + list[i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store) } } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromList(list), }) return @@ -224,36 +226,36 @@ func UverseNode() *PackageNode { // ------------------------------------------------------------ // append(nil, *NativeValue) case *NativeValue: - argsrv := args.Value - argsl := argsrv.Len() - if argsl == 0 { // no change + arg1NativeValue := arg1Value.Value + arg1NativeValueLength := arg1NativeValue.Len() + if arg1NativeValueLength == 0 { // no change m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: nil, }) return - } else if xt.Elem().Kind() == Uint8Kind { + } else if arg0Type.Elem().Kind() == Uint8Kind { // append(nil, *NativeValue) new data bytes -- - data := make([]byte, argsl) + data := make([]byte, arg1NativeValueLength) copyNativeToData( - data[:argsl], - argsrv, argsl) + data[:arg1NativeValueLength], + arg1NativeValue, arg1NativeValueLength) m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromData(data), }) return } else { // append(nil, *NativeValue) new list -------- - list := make([]TypedValue, argsl) - if 0 < argsl { + list := make([]TypedValue, arg1NativeValueLength) + if 0 < arg1NativeValueLength { copyNativeToList( m.Alloc, - list[:argsl], - argsrv, argsl) + list[:arg1NativeValueLength], + arg1NativeValue, arg1NativeValueLength) } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromList(list), }) return @@ -267,143 +269,137 @@ func UverseNode() *PackageNode { // ---------------------------------------------------------------- // append(*SliceValue, ???) case *SliceValue: - xvl := xv.Length - xvo := xv.Offset - xvc := xv.Maxcap - xvb := xv.GetBase(m.Store) - switch args := arg1.TV.V.(type) { + arg0Length := arg0Value.Length + arg0Offset := arg0Value.Offset + arg0Capacity := arg0Value.Maxcap + arg0Base := arg0Value.GetBase(m.Store) + switch arg1Value := arg1.TV.V.(type) { // ------------------------------------------------------------ // append(*SliceValue, nil) case nil: // no change m.PushValue(TypedValue{ - T: xt, - V: xv, + T: arg0Type, + V: arg0Value, }) return // ------------------------------------------------------------ // append(*SliceValue, *SliceValue) case *SliceValue: - argsl := args.Length - argso := args.Offset - argsb := args.GetBase(m.Store) - if xvl+argsl <= xvc { + arg1Length := arg1Value.Length + arg1Offset := arg1Value.Offset + arg1Base := arg1Value.GetBase(m.Store) + if arg0Length+arg1Length <= arg0Capacity { // append(*SliceValue, *SliceValue) w/i capacity ----- - if 0 < argsl { // implies 0 < xvc - if xvb.Data == nil { + if 0 < arg1Length { // implies 0 < xvc + if arg0Base.Data == nil { // append(*SliceValue.List, *SliceValue) --------- - list := xvb.List - if argsb.Data == nil { - for i := 0; i < argsl; i++ { - oldElem := list[xvo+xvl+i] + list := arg0Base.List + if arg1Base.Data == nil { + for i := 0; i < arg1Length; i++ { + oldElem := list[arg0Offset+arg0Length+i] // unrefCopy will resolve references and copy their values // to copy by value rather than by reference. - newElem := argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) - list[xvo+xvl+i] = newElem + newElem := arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store) + list[arg0Offset+arg0Length+i] = newElem m.Realm.DidUpdate( - xvb, + arg0Base, oldElem.GetFirstObject(m.Store), newElem.GetFirstObject(m.Store), ) } } else { copyDataToList( - list[xvo+xvl:xvo+xvl+argsl], - argsb.Data[argso:argso+argsl], - xt.Elem()) - m.Realm.DidUpdate(xvb, nil, nil) + list[arg0Offset+arg0Length:arg0Offset+arg0Length+arg1Length], + arg1Base.Data[arg1Offset:arg1Offset+arg1Length], + arg0Type.Elem()) + m.Realm.DidUpdate(arg1Base, nil, nil) } } else { // append(*SliceValue.Data, *SliceValue) --------- - data := xvb.Data - if argsb.Data == nil { + data := arg0Base.Data + if arg1Base.Data == nil { copyListToData( - data[xvo+xvl:xvo+xvl+argsl], - argsb.List[argso:argso+argsl]) - m.Realm.DidUpdate(xvb, nil, nil) + data[arg0Offset+arg0Length:arg0Offset+arg0Length+arg1Length], + arg1Base.List[arg1Offset:arg1Offset+arg1Length]) + m.Realm.DidUpdate(arg0Base, nil, nil) } else { copy( - data[xvo+xvl:xvo+xvl+argsl], - argsb.Data[argso:argso+argsl]) + data[arg0Offset+arg0Length:arg0Offset+arg0Length+arg1Length], + arg1Base.Data[arg1Offset:arg1Offset+arg1Length]) } } m.PushValue(TypedValue{ - T: xt, - V: m.Alloc.NewSlice(xvb, xvo, xvl+argsl, xvc), + T: arg0Type, + V: m.Alloc.NewSlice(arg0Base, arg0Offset, arg0Length+arg1Length, arg0Capacity), }) return } else { // no change m.PushValue(TypedValue{ - T: xt, - V: xv, + T: arg0Type, + V: arg0Value, }) return } - } else if xt.Elem().Kind() == Uint8Kind { + } else if arg0Type.Elem().Kind() == Uint8Kind { // append(*SliceValue, *SliceValue) new data bytes --- - data := make([]byte, xvl+argsl) - if 0 < xvl { - if xvb.Data == nil { + data := make([]byte, arg0Length+arg1Length) + if 0 < arg0Length { + if arg0Base.Data == nil { copyListToData( - data[:xvl], - xvb.List[xvo:xvo+xvl]) + data[:arg0Length], + arg0Base.List[arg0Offset:arg0Offset+arg0Length]) } else { copy( - data[:xvl], - xvb.Data[xvo:xvo+xvl]) + data[:arg0Length], + arg0Base.Data[arg0Offset:arg0Offset+arg0Length]) } } - if 0 < argsl { - if argsb.Data == nil { + if 0 < arg1Length { + if arg1Base.Data == nil { copyListToData( - data[xvl:xvl+argsl], - argsb.List[argso:argso+argsl]) + data[arg0Length:arg0Length+arg1Length], + arg1Base.List[arg1Offset:arg1Offset+arg1Length]) } else { copy( - data[xvl:xvl+argsl], - argsb.Data[argso:argso+argsl]) + data[arg0Length:arg0Length+arg1Length], + arg1Base.Data[arg1Offset:arg1Offset+arg1Length]) } } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromData(data), }) return } else { // append(*SliceValue, *SliceValue) new list --------- - list := make([]TypedValue, xvl+argsl) - if 0 < xvl { - if xvb.Data == nil { - for i := 0; i < xvl; i++ { - list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) + list := make([]TypedValue, arg0Length+arg1Length) + if 0 < arg0Length { + if arg0Base.Data == nil { + for i := 0; i < arg0Length; i++ { + list[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store) } } else { panic("should not happen") - /* - copyDataToList( - list[:xvl], - xvb.Data[xvo:xvo+xvl], - xt.Elem(), - ) - */ } } - if 0 < argsl { - if argsb.Data == nil { - for i := 0; i < argsl; i++ { - list[xvl+i] = argsb.List[argso+i].unrefCopy(m.Alloc, m.Store) + + if 0 < arg1Length { + if arg1Base.Data == nil { + for i := 0; i < arg1Length; i++ { + list[arg0Length+i] = arg1Base.List[arg1Offset+i].unrefCopy(m.Alloc, m.Store) } } else { copyDataToList( - list[xvl:xvl+argsl], - argsb.Data[argso:argso+argsl], - argt.Elem(), + list[arg0Length:arg0Length+arg1Length], + arg1Base.Data[arg1Offset:arg1Offset+arg1Length], + arg1Type.Elem(), ) } } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromList(list), }) return @@ -412,78 +408,78 @@ func UverseNode() *PackageNode { // ------------------------------------------------------------ // append(*SliceValue, *NativeValue) case *NativeValue: - argsrv := args.Value - argsl := argsrv.Len() - if xvl+argsl <= xvc { + arg1NativeValue := arg1Value.Value + arg1NativeValueLength := arg1NativeValue.Len() + if arg0Length+arg1NativeValueLength <= arg0Capacity { // append(*SliceValue, *NativeValue) w/i capacity ---- - if 0 < argsl { // implies 0 < xvc - if xvb.Data == nil { + if 0 < arg1NativeValueLength { // implies 0 < xvc + if arg0Base.Data == nil { // append(*SliceValue.List, *NativeValue) -------- - list := xvb.List + list := arg0Base.List copyNativeToList( m.Alloc, - list[xvo:xvo+argsl], - argsrv, argsl) + list[arg0Offset:arg0Offset+arg1NativeValueLength], + arg1NativeValue, arg1NativeValueLength) } else { // append(*SliceValue.Data, *NativeValue) -------- - data := xvb.Data + data := arg0Base.Data copyNativeToData( - data[xvo:xvo+argsl], - argsrv, argsl) + data[arg0Offset:arg0Offset+arg1NativeValueLength], + arg1NativeValue, arg1NativeValueLength) } m.PushValue(TypedValue{ - T: xt, - V: m.Alloc.NewSlice(xvb, xvo, xvl+argsl, xvc), + T: arg0Type, + V: m.Alloc.NewSlice(arg0Base, arg0Offset, arg0Length+arg1NativeValueLength, arg0Capacity), }) return } else { // no change m.PushValue(TypedValue{ - T: xt, - V: xv, + T: arg0Type, + V: arg0Value, }) return } - } else if xt.Elem().Kind() == Uint8Kind { + } else if arg0Type.Elem().Kind() == Uint8Kind { // append(*SliceValue, *NativeValue) new data bytes -- - data := make([]byte, xvl+argsl) - if 0 < xvl { - if xvb.Data == nil { + data := make([]byte, arg0Length+arg1NativeValueLength) + if 0 < arg0Length { + if arg0Base.Data == nil { copyListToData( - data[:xvl], - xvb.List[xvo:xvo+xvl]) + data[:arg0Length], + arg0Base.List[arg0Offset:arg0Offset+arg0Length]) } else { copy( - data[:xvl], - xvb.Data[xvo:xvo+xvl]) + data[:arg0Length], + arg0Base.Data[arg0Offset:arg0Offset+arg0Length]) } } - if 0 < argsl { + if 0 < arg1NativeValueLength { copyNativeToData( - data[xvl:xvl+argsl], - argsrv, argsl) + data[arg0Length:arg0Length+arg1NativeValueLength], + arg1NativeValue, arg1NativeValueLength) } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromData(data), }) return } else { // append(*SliceValue, *NativeValue) new list -------- - listLen := xvl + argsl + listLen := arg0Length + arg1NativeValueLength list := make([]TypedValue, listLen) - if 0 < xvl { + if 0 < arg0Length { for i := 0; i < listLen; i++ { - list[i] = xvb.List[xvo+i].unrefCopy(m.Alloc, m.Store) + list[i] = arg0Base.List[arg0Offset+i].unrefCopy(m.Alloc, m.Store) } } - if 0 < argsl { + if 0 < arg1NativeValueLength { copyNativeToList( m.Alloc, - list[xvl:xvl+argsl], - argsrv, argsl) + list[arg0Length:listLen], + arg1NativeValue, arg1NativeValueLength) } m.PushValue(TypedValue{ - T: xt, + T: arg0Type, V: m.Alloc.NewSliceFromList(list), }) return @@ -497,51 +493,51 @@ func UverseNode() *PackageNode { // ---------------------------------------------------------------- // append(*NativeValue, ???) case *NativeValue: - sv := xv.Value - switch args := arg1.TV.V.(type) { + arg0NativeValue := arg0Value.Value + switch arg1Value := arg1.TV.V.(type) { // ------------------------------------------------------------ // append(*NativeValue, nil) case nil: // no change m.PushValue(TypedValue{ - T: xt, - V: xv, + T: arg0Type, + V: arg0Value, }) return // ------------------------------------------------------------ // append(*NativeValue, *SliceValue) case *SliceValue: - st := sv.Type() - argso := args.Offset - argsl := args.Length - argsb := args.GetBase(m.Store) - if 0 < argsl { - argsrv := reflect.MakeSlice(st, argsl, argsl) - if argsb.Data == nil { - for i := 0; i < argsl; i++ { - etv := &(argsb.List[argso+i]) + arg0NativeValueType := arg0NativeValue.Type() + arg1Offset := arg1Value.Offset + arg1Length := arg1Value.Length + arg1Base := arg1Value.GetBase(m.Store) + if 0 < arg1Length { + newNativeArg1Slice := reflect.MakeSlice(arg0NativeValueType, arg1Length, arg1Length) + if arg1Base.Data == nil { + for i := 0; i < arg1Length; i++ { + etv := &(arg1Base.List[arg1Offset+i]) if etv.IsUndefined() { continue } erv := gno2GoValue(etv, reflect.Value{}) - argsrv.Index(i).Set(erv) + newNativeArg1Slice.Index(i).Set(erv) } } else { - for i := 0; i < argsl; i++ { - erv := argsrv.Index(i) - erv.SetUint(uint64(argsb.Data[argso+i])) + for i := 0; i < arg1Length; i++ { + erv := newNativeArg1Slice.Index(i) + erv.SetUint(uint64(arg1Base.Data[arg1Offset+i])) } } - resrv := reflect.AppendSlice(sv, argsrv) + modifiedNativeSlice := reflect.AppendSlice(arg0NativeValue, newNativeArg1Slice) m.PushValue(TypedValue{ - T: xt, - V: m.Alloc.NewNative(resrv), + T: arg0Type, + V: m.Alloc.NewNative(modifiedNativeSlice), }) return } else { // no change m.PushValue(TypedValue{ - T: xt, - V: xv, + T: arg0Type, + V: arg0Value, }) return } @@ -549,31 +545,31 @@ func UverseNode() *PackageNode { // ------------------------------------------------------------ // append(*NativeValue, *NativeValue) case *NativeValue: - argsrv := args.Value - resrv := reflect.AppendSlice(sv, argsrv) + arg1ReflectValue := arg1Value.Value + modifiedNativeSlice := reflect.AppendSlice(arg0NativeValue, arg1ReflectValue) m.PushValue(TypedValue{ - T: xt, - V: m.Alloc.NewNative(resrv), + T: arg0Type, + V: m.Alloc.NewNative(modifiedNativeSlice), }) return // ------------------------------------------------------------ // append(*NativeValue, StringValue) case StringValue: - if xt.Elem().Kind() == Uint8Kind { + if arg0Type.Elem().Kind() == Uint8Kind { // TODO this might be faster if reflect supports // appending this way without first converting to a slice. - argrv := reflect.ValueOf([]byte(arg1.TV.GetString())) - resrv := reflect.AppendSlice(sv, argrv) + arg1ReflectValue := reflect.ValueOf([]byte(arg1.TV.GetString())) + modifiedNativeSlice := reflect.AppendSlice(arg0NativeValue, arg1ReflectValue) m.PushValue(TypedValue{ - T: xt, - V: m.Alloc.NewNative(resrv), + T: arg0Type, + V: m.Alloc.NewNative(modifiedNativeSlice), }) return } else { panic(fmt.Sprintf( "cannot append %s to %s", - arg1.TV.T.String(), xt.String())) + arg1.TV.T.String(), arg0Type.String())) } // ------------------------------------------------------------ @@ -581,7 +577,7 @@ func UverseNode() *PackageNode { default: panic(fmt.Sprintf( "cannot append %s to %s", - arg1.TV.T.String(), xt.String())) + arg1.TV.T.String(), arg0Type.String())) } // ----------------------------------------------------------------