Fix new vdl convert tests to work again.
A variety of changes, including float64/float32 conversions,
retaining reflect type info in ValueFromReflect, and fixing a
MimicValue bug wrt zero base fields.
Change-Id: I35f257766f66fd9bf916e8567f8595c16216a22e
diff --git a/vdl/new_convert_test.go b/vdl/new_convert_test.go
index 7b03157..57573a5 100644
--- a/vdl/new_convert_test.go
+++ b/vdl/new_convert_test.go
@@ -18,10 +18,10 @@
for _, entry := range vdltest.AllPass() {
rvTargetPtr := reflect.New(entry.Target.Type())
if err := vdl.Convert(rvTargetPtr.Interface(), entry.Source.Interface()); err != nil {
- t.Errorf("%s: error %v", entry.Name(), err)
+ t.Errorf("%s: Convert failed: %v", entry.Name(), err)
}
- if !vdl.DeepEqualReflect(rvTargetPtr.Elem(), entry.Target) {
- t.Errorf("%[1]s:\nGOT %[2]v\nWANT %[3]v", entry.Name(), rvTargetPtr.Elem(), entry.Target)
+ if got, want := rvTargetPtr.Elem(), entry.Target; !vdl.DeepEqualReflect(got, want) {
+ t.Errorf("%s\nGOT %v\nWANT %v", entry.Name(), got, want)
}
}
}
@@ -30,7 +30,7 @@
for _, entry := range vdltest.AllFail() {
rvTargetPtr := reflect.New(entry.Target.Type())
if err := vdl.Convert(rvTargetPtr.Interface(), entry.Source.Interface()); err == nil {
- t.Errorf("%s: expected failure", entry.Name())
+ t.Errorf("%s: Convert passed, wanted failure", entry.Name())
}
}
}
diff --git a/vdl/new_value_reader_test.go b/vdl/new_value_reader_test.go
index 4ada3cd..fed2366 100644
--- a/vdl/new_value_reader_test.go
+++ b/vdl/new_value_reader_test.go
@@ -13,15 +13,15 @@
"v.io/v23/vdl/vdltest"
)
-func TestValueReadNew(t *testing.T) {
+func TestValueReader(t *testing.T) {
for _, entry := range vdltest.ToEntryValues(vdltest.AllPass()) {
out := vdl.ZeroValue(entry.Target.Type())
if err := out.VDLRead(entry.Source.Decoder()); err != nil {
- t.Errorf("%s: error in ValueRead: %v", entry.Name(), err)
+ t.Errorf("%s: VDLRead failed: %v", entry.Name(), err)
continue
}
- if !vdl.EqualValue(entry.Target, out) {
- t.Errorf("%s: got %v, want %v", entry.Name(), out, entry.Target)
+ if got, want := out, entry.Target; !vdl.EqualValue(got, want) {
+ t.Errorf("%s\nGOT %v\nWANT %v", entry.Name(), got, want)
}
}
}
diff --git a/vdl/pipe.go b/vdl/pipe.go
index cebaa18..0daea79 100644
--- a/vdl/pipe.go
+++ b/vdl/pipe.go
@@ -8,6 +8,7 @@
"errors"
"fmt"
"math"
+ "reflect"
"sync"
)
@@ -25,6 +26,14 @@
return dec.Close(Read(dec, dst))
}
+func convertPipeReflect(dst, src reflect.Value) error {
+ enc, dec := newPipe()
+ go func() {
+ enc.Close(WriteReflect(enc, src))
+ }()
+ return dec.Close(ReadReflect(dec, dst))
+}
+
type pipeStackEntry struct {
Type *Type
NextOp pipeOp
diff --git a/vdl/target.go b/vdl/target.go
index 517ebb3..cc9af88 100644
--- a/vdl/target.go
+++ b/vdl/target.go
@@ -172,7 +172,7 @@
return ZeroValue(AnyType), nil
}
var result *Value
- err := convertPipe(&result, rv.Interface())
+ err := convertPipeReflect(reflect.ValueOf(&result), rv)
return result, err
}
diff --git a/vdl/value.go b/vdl/value.go
index 979428c..20c0a8c 100644
--- a/vdl/value.go
+++ b/vdl/value.go
@@ -354,7 +354,15 @@
case int64:
return arep == b.rep.(int64)
case float64:
- return arep == b.rep.(float64)
+ // Float32 is represented as float64, but we must convert to float32 for
+ // equality comparisons. Otherwise a single float32 may be represented as
+ // different values, causing equality failures.
+ switch a.t.kind {
+ case Float32:
+ return float32(arep) == float32(b.rep.(float64))
+ default:
+ return arep == b.rep.(float64)
+ }
case string:
return arep == b.rep.(string)
case enumIndex:
diff --git a/vdl/value_rep.go b/vdl/value_rep.go
index d481fdf..a83151c 100644
--- a/vdl/value_rep.go
+++ b/vdl/value_rep.go
@@ -118,8 +118,14 @@
// fastKeyRep returns a representation of key that may be used in a regular Go
// map. It's only called on keys where useFastIndex returns true.
func fastKeyRep(key *Value) interface{} {
- if key.t.IsBytes() {
- // Turn []byte into a string so that it can be used as a map key.
+ switch {
+ case key.Kind() == Float32:
+ // Float32 is represented as float64 in vdl.Value, but we must convert to
+ // float32 to ensure map lookups work with the correct precision. Otherwise
+ // a single float32 may be reprsesented as different keys.
+ return float32(key.rep.(float64))
+ case key.t.IsBytes():
+ // Convert []byte into a string so that it can be used as a map key.
return string(key.Bytes())
}
return key.rep
diff --git a/vdl/vdltest/.api b/vdl/vdltest/.api
index 5bfc25c..9844374 100644
--- a/vdl/vdltest/.api
+++ b/vdl/vdltest/.api
@@ -1660,7 +1660,9 @@
pkg vdltest, type EntryValue struct
pkg vdltest, type EntryValue struct, Label string
pkg vdltest, type EntryValue struct, Source *vdl.Value
+pkg vdltest, type EntryValue struct, SourceLabel string
pkg vdltest, type EntryValue struct, Target *vdl.Value
+pkg vdltest, type EntryValue struct, TargetLabel string
pkg vdltest, type GenMode int
pkg vdltest, type TypeGenerator struct
pkg vdltest, type TypeGenerator struct, BaseTypesPerKind []int
diff --git a/vdl/vdltest/entry.go b/vdl/vdltest/entry.go
index 4ca27c6..9d55eba 100644
--- a/vdl/vdltest/entry.go
+++ b/vdl/vdltest/entry.go
@@ -36,14 +36,24 @@
// EntryValue is like Entry, but represents the target and source values as
// *vdl.Value, rather than interface{}.
type EntryValue struct {
- Label string
- Target *vdl.Value
- Source *vdl.Value
+ Label string
+ TargetLabel string
+ Target *vdl.Value
+ SourceLabel string
+ Source *vdl.Value
}
// Name returns the name of the EntryValue.
func (e EntryValue) Name() string {
- return e.Label + " Target(" + e.Target.String() + ") Source(" + e.Source.String() + ")"
+ tLabel := e.TargetLabel
+ if tLabel == "" {
+ tLabel = e.Target.String()
+ }
+ sLabel := e.SourceLabel
+ if sLabel == "" {
+ sLabel = e.Source.String()
+ }
+ return e.Label + " Target(" + tLabel + ") Source(" + sLabel + ")"
}
// ToEntryValue converts the Entry e into an EntryValue.
@@ -57,9 +67,11 @@
panic(err)
}
return EntryValue{
- Label: e.Label,
- Target: target,
- Source: source,
+ Label: e.Label,
+ TargetLabel: e.TargetLabel,
+ Target: target,
+ SourceLabel: e.SourceLabel,
+ Source: source,
}
}
diff --git a/vdl/vdltest/entry_generator.go b/vdl/vdltest/entry_generator.go
index f786ad3..4e85040 100644
--- a/vdl/vdltest/entry_generator.go
+++ b/vdl/vdltest/entry_generator.go
@@ -146,7 +146,11 @@
ev := g.genPass("Zero", vdl.ZeroValue(tt), g.ZeroEntryLimit, sourceAll)
if tt.Kind() == vdl.Optional {
// Add entry to convert from any(nil) to optional(nil).
- ev = append(ev, EntryValue{"NilAny", vdl.ZeroValue(tt), vdl.ZeroValue(vdl.AnyType)})
+ ev = append(ev, EntryValue{
+ Label: "NilAny",
+ Target: vdl.ZeroValue(tt),
+ Source: vdl.ZeroValue(vdl.AnyType),
+ })
}
// Handle some special-cases.
switch {
@@ -256,7 +260,11 @@
var ev []EntryValue
if mode.Canonical() {
// Add the canonical identity conversion for each target value.
- ev = append(ev, EntryValue{label, target, target})
+ ev = append(ev, EntryValue{
+ Label: label,
+ Target: target,
+ Source: target,
+ })
}
// Add up to limit conversion entries. The strategy is to add an entry for
// each source type where we can create a value that can convert to the
@@ -286,7 +294,11 @@
break
}
num++
- ev = append(ev, EntryValue{label, target, source})
+ ev = append(ev, EntryValue{
+ Label: label,
+ Target: target,
+ Source: source,
+ })
}
}
return ev
diff --git a/vdl/vdltest/mimic_value.go b/vdl/vdltest/mimic_value.go
index e34cee3..70d3aed 100644
--- a/vdl/vdltest/mimic_value.go
+++ b/vdl/vdltest/mimic_value.go
@@ -315,15 +315,15 @@
value := vdl.ZeroValue(tt)
for ix := 0; ix < base.Type().NumField(); ix++ {
baseField := base.StructField(ix)
- if baseField.IsZero() {
- // Skip zero base fields. It's fine for the base field to be missing from
- // tt, as long as the base field is zero, since Convert(dst, src) sets all
- // dst (which has type base.Type) fields to zero before setting each
- // matching field in src (which has type tt).
- continue
- }
ttField, ttIndex := tt.FieldByName(base.Type().Field(ix).Name)
if ttIndex == -1 {
+ if baseField.IsZero() {
+ // Skip zero base fields. It's fine for the base field to be missing
+ // from tt, as long as the base field is zero, since Convert(dst, src)
+ // sets all dst (which has type base.Type) fields to zero before setting
+ // each matching field in src (which has type tt).
+ continue
+ }
// This is a non-zero base field that doesn't exist in tt. There's no way
// to create a value of type tt that converts to exactly the base value.
return nil
diff --git a/vdl/vdltest/vdltest.vdl.go b/vdl/vdltest/vdltest.vdl.go
index df9c33c..48e0bef 100644
--- a/vdl/vdltest/vdltest.vdl.go
+++ b/vdl/vdltest/vdltest.vdl.go
@@ -39270,25 +39270,15 @@
Label: "Zero",
TargetLabel: "VStructDepth1_Any{}",
Target: VStructDepth1_Any{},
- SourceLabel: "VStructDepth2_All{}",
- Source: VStructDepth2_All{
- F0: VArray2_TypeObject{
- vdl.AnyType,
- vdl.AnyType,
- },
- F8: VStructDepth1_All{
- F5: vdl.AnyType,
- },
- F10: VUnionDepth1_AllF0{},
- F11: VUnionDepth1_Float64F29{},
- },
+ SourceLabel: "?VStructEmpty{}",
+ Source: &VStructEmpty{},
},
{
Label: "Zero",
TargetLabel: "VStructDepth1_Any{}",
Target: VStructDepth1_Any{},
- SourceLabel: "?VStructEmpty{}",
- Source: &VStructEmpty{},
+ SourceLabel: "VStructEmpty{}",
+ Source: VStructEmpty{},
},
{
IsCanonical: true,
@@ -47546,8 +47536,12 @@
vdl.AnyType,
},
},
- SourceLabel: "VStructDepth1_All{}",
+ SourceLabel: "VStructDepth1_All{F0: VArray2_TypeObject{}}",
Source: VStructDepth1_All{
+ F0: VArray2_TypeObject{
+ vdl.AnyType,
+ vdl.AnyType,
+ },
F5: vdl.AnyType,
},
},
@@ -47560,8 +47554,13 @@
vdl.AnyType,
},
},
- SourceLabel: "VStructDepth1_Any{}",
- Source: VStructDepth1_Any{},
+ SourceLabel: "VStructDepth1_Any{F0: VArray2_TypeObject{}}",
+ Source: VStructDepth1_Any{
+ F0: VArray2_TypeObject{
+ vdl.AnyType,
+ vdl.AnyType,
+ },
+ },
},
{
IsCanonical: true,
@@ -57467,8 +57466,10 @@
Label: "Zero",
TargetLabel: "VStructDepth3_VArray1_Set_VEnumBcd{}",
Target: VStructDepth3_VArray1_Set_VEnumBcd{},
- SourceLabel: "VStructDepth1_Any{}",
- Source: VStructDepth1_Any{},
+ SourceLabel: "VStructDepth1_Any{F0: VArray1_Set_VEnumBcd{}}",
+ Source: VStructDepth1_Any{
+ F0: VArray1_Set_VEnumBcd{},
+ },
},
{
IsCanonical: true,
@@ -74904,28 +74905,17 @@
Label: "Zero",
TargetLabel: "XStructDepth1_Any{}",
Target: XStructDepth1_Any{},
- SourceLabel: "?XStructDepth3_XArray1_Set_XEnumBcd{}",
- Source: &XStructDepth3_XArray1_Set_XEnumBcd{},
+ SourceLabel: "XStructDepth1_All{}",
+ Source: XStructDepth1_All{
+ F5: vdl.AnyType,
+ },
},
{
Label: "Zero",
TargetLabel: "XStructDepth1_Any{}",
Target: XStructDepth1_Any{},
- SourceLabel: "XStructDepth3_All{}",
- Source: XStructDepth3_All{
- F4: XStructDepth2_All{
- F0: XArray2_TypeObject{
- vdl.AnyType,
- vdl.AnyType,
- },
- F8: XStructDepth1_All{
- F5: vdl.AnyType,
- },
- F10: XUnionDepth1_AllF0{},
- F11: XUnionDepth1_Float64F29{},
- },
- F5: XUnionDepth2_AllF0{},
- },
+ SourceLabel: "?XStructEmpty{}",
+ Source: &XStructEmpty{},
},
{
IsCanonical: true,
diff --git a/vdl/vdltest/ventry_pass_gen.vdl b/vdl/vdltest/ventry_pass_gen.vdl
index 6ee1ac6..fa285f0 100644
--- a/vdl/vdltest/ventry_pass_gen.vdl
+++ b/vdl/vdltest/ventry_pass_gen.vdl
@@ -12,8 +12,8 @@
// ------------------------------------------------------------------------------------------
// | | Total | +Max | +Min |-Max |-Min | Full |NilAny|Random | Zero |isZero |
// ------------------------------------------------------------------------------------------
-// |total |1040,1717|23,141|23,226|15,65|15,94|349,331| 0, 8|263,240|352,612|352,452| [!can max=14]
-// |isZero | 352, 452| 0, 0| 0, 0| 0, 0| 0, 0| 0, 2| 0, 8| 0, 0|352,442|352,452| [!can max=3]
+// |total |1040,1717|23,141|23,226|15,65|15,94|349,331| 0, 8|263,240|352,612|352,449| [!can max=14]
+// |isZero | 352, 449| 0, 0| 0, 0| 0, 0| 0, 0| 0, 2| 0, 8| 0, 0|352,439|352,449| [!can max=3]
// ------------------------------------------------------------------------------------------
// |any | 1, 8| 0, 0| 0, 0| 0, 0| 0, 0| 0, 0| 0, 8| 0, 0| 1, 0| 1, 8| [nil=9]
// |optional | 21, 62| 0, 0| 0, 0| 0, 0| 0, 0| 8, 4| 0, 0| 5, 4| 8, 54| 8, 11| [!can max=3] [nil=19]
@@ -35,10 +35,10 @@
// |list | 176, 348| 0, 0| 0, 0| 0, 0| 0, 0| 66, 93| 0, 0| 44, 62| 66,193| 66,132| [!can max=2] [len max=3]
// |set | 100, 116| 0, 0| 0, 0| 0, 0| 0, 0| 38, 32| 0, 0| 24, 20| 38, 64| 38, 64| [!can max=2] [len max=3]
// |map | 102, 106| 0, 0| 0, 0| 0, 0| 0, 0| 38, 28| 0, 0| 26, 16| 38, 62| 38, 62| [!can max=2] [len max=3]
-// |struct | 193, 208| 0, 0| 0, 0| 0, 0| 0, 0| 67, 63| 0, 0| 57, 50| 69, 95| 69, 96| [!can max=2]
+// |struct | 193, 208| 0, 0| 0, 0| 0, 0| 0, 0| 67, 63| 0, 0| 57, 50| 69, 95| 69, 93| [!can max=2]
// |union | 177, 187| 0, 0| 0, 0| 0, 0| 0, 0| 62, 62| 0, 0| 52, 52| 63, 73| 63, 16| [!can max=2]
// ------------------------------------------------------------------------------------------
-// |IsNamed | 761,1075|13, 78|13,128| 9,37| 9,56|258,226| 0, 0|199,180|260,370|260,277| [!can max=14]
+// |IsNamed | 761,1075|13, 78|13,128| 9,37| 9,56|258,226| 0, 0|199,180|260,370|260,274| [!can max=14]
// |IsUnnamed | 279, 642|10, 63|10, 98| 6,28| 6,38| 91,105| 0, 8| 64, 60| 92,242| 92,175| [!can max=11]
// |IsBytes | 18, 15| 0, 0| 0, 0| 0, 0| 0, 0| 6, 4| 0, 0| 6, 2| 6, 9| 6, 3| [!can max=2]
// |IsPartOfCycle| 9, 4| 0, 0| 0, 0| 0, 0| 0, 0| 3, 0| 0, 0| 3, 0| 3, 4| 3, 4| [!can max=2]
@@ -3352,8 +3352,8 @@
},
// Canonical
{ true, `Zero`, `VStructDepth1_Any{}`, VStructDepth1_Any{}, `VStructDepth1_Any{}`, VStructDepth1_Any{} },
- { false, `Zero`, `VStructDepth1_Any{}`, VStructDepth1_Any{}, `VStructDepth2_All{}`, VStructDepth2_All{} },
{ false, `Zero`, `VStructDepth1_Any{}`, VStructDepth1_Any{}, `?VStructEmpty{}`, ?VStructEmpty{} },
+ { false, `Zero`, `VStructDepth1_Any{}`, VStructDepth1_Any{}, `VStructEmpty{}`, VStructEmpty{} },
// Canonical
{ true, `Full`,
`VStructDepth1_Any{F0: int64(-123)}`,
@@ -6469,14 +6469,14 @@
{ false, `Zero`,
`VStructDepth2_VArray2_TypeObject{}`,
VStructDepth2_VArray2_TypeObject{},
- `VStructDepth1_All{}`,
- VStructDepth1_All{},
+ `VStructDepth1_All{F0: VArray2_TypeObject{}}`,
+ VStructDepth1_All{F0: VArray2_TypeObject{}},
},
{ false, `Zero`,
`VStructDepth2_VArray2_TypeObject{}`,
VStructDepth2_VArray2_TypeObject{},
- `VStructDepth1_Any{}`,
- VStructDepth1_Any{},
+ `VStructDepth1_Any{F0: VArray2_TypeObject{}}`,
+ VStructDepth1_Any{F0: VArray2_TypeObject{}},
},
// Canonical
{ true, `Full`,
@@ -8730,8 +8730,8 @@
{ false, `Zero`,
`VStructDepth3_VArray1_Set_VEnumBcd{}`,
VStructDepth3_VArray1_Set_VEnumBcd{},
- `VStructDepth1_Any{}`,
- VStructDepth1_Any{},
+ `VStructDepth1_Any{F0: VArray1_Set_VEnumBcd{}}`,
+ VStructDepth1_Any{F0: VArray1_Set_VEnumBcd{}},
},
// Canonical
{ true, `Full`,
diff --git a/vdl/vdltest/xentry_pass_gen.vdl b/vdl/vdltest/xentry_pass_gen.vdl
index 19e9d27..f994f92 100644
--- a/vdl/vdltest/xentry_pass_gen.vdl
+++ b/vdl/vdltest/xentry_pass_gen.vdl
@@ -2498,13 +2498,8 @@
},
// Canonical
{ true, `Zero`, `XStructDepth1_Any{}`, XStructDepth1_Any{}, `XStructDepth1_Any{}`, XStructDepth1_Any{} },
- { false, `Zero`,
- `XStructDepth1_Any{}`,
- XStructDepth1_Any{},
- `?XStructDepth3_XArray1_Set_XEnumBcd{}`,
- ?XStructDepth3_XArray1_Set_XEnumBcd{},
- },
- { false, `Zero`, `XStructDepth1_Any{}`, XStructDepth1_Any{}, `XStructDepth3_All{}`, XStructDepth3_All{} },
+ { false, `Zero`, `XStructDepth1_Any{}`, XStructDepth1_Any{}, `XStructDepth1_All{}`, XStructDepth1_All{} },
+ { false, `Zero`, `XStructDepth1_Any{}`, XStructDepth1_Any{}, `?XStructEmpty{}`, ?XStructEmpty{} },
// Canonical
{ true, `Full`,
`XStructDepth1_Any{F0: int64(-123)}`,
diff --git a/vom/xdecoder_test.go b/vom/xdecoder_test.go
index ff885ef..7d3db6b 100644
--- a/vom/xdecoder_test.go
+++ b/vom/xdecoder_test.go
@@ -41,7 +41,7 @@
vv, err := vdl.ValueFromReflect(test.Value)
if err != nil {
t.Errorf("%s: ValueFromReflect failed: %v", test.Name(), err)
- continue
+ return
}
vvWant := reflect.ValueOf(vv)
testXDecoder(t, "[new *vdl.Value]", test, vvWant)
