core.go: The vdl "any" type is now generated as go *vdl.Value.
Previously we were generating as vdl.AnyRep, which was another
name for interface{}. There are multiple problems with using
interface{}:
1) Decoding into an "interface{}" requires that the type has
been registered with VDL, and uses our type name based
matching logic. This is error prone; if we happen to change
the package path, the type name changes. And if the type
isn't registered, we'll end up with a *vdl.Value anyways.
Now there's no ambiguity; you always get a *vdl.Value, which
can represent values of all vdl types.
2) Some Go values aren't valid in VDL. E.g. Go values that
contain channels, functions or unsafe pointers are all
invalid. Using *vdl.Value at our API boundaries forces the
callers to make an informed decision.
This is the main portion of a 7-part CL, which removes
vdl.AnyRep, and uses *vdl.Value instead.
MultiPart: 1/7
Change-Id: Ie4f3ab6bce0363f82f53c2315a7435844f6ffe85
diff --git a/services/mgmt/debug/dispatcher_test.go b/services/mgmt/debug/dispatcher_test.go
index 09e8f6b..8c59e3c 100644
--- a/services/mgmt/debug/dispatcher_test.go
+++ b/services/mgmt/debug/dispatcher_test.go
@@ -19,6 +19,7 @@
"v.io/core/veyron2/services/mgmt/logreader"
"v.io/core/veyron2/services/mgmt/stats"
vtracesvc "v.io/core/veyron2/services/mgmt/vtrace"
+ "v.io/core/veyron2/vdl"
"v.io/core/veyron2/verror"
"v.io/core/veyron2/vtrace"
@@ -127,8 +128,8 @@
if err != nil {
t.Errorf("Value failed: %v", err)
}
- if expected := int64(123); v != expected {
- t.Errorf("unexpected result. Got %v, want %v", v, expected)
+ if want := vdl.Int64Value(123); !vdl.EqualValue(v, want) {
+ t.Errorf("unexpected result. got %v, want %v", v, want)
}
}
diff --git a/services/mgmt/device/impl/instance_reaping.go b/services/mgmt/device/impl/instance_reaping.go
index 50aa130..d59c927 100644
--- a/services/mgmt/device/impl/instance_reaping.go
+++ b/services/mgmt/device/impl/instance_reaping.go
@@ -1,6 +1,7 @@
package impl
import (
+ "fmt"
"path/filepath"
"sync"
"syscall"
@@ -9,6 +10,7 @@
"v.io/core/veyron2/context"
"v.io/core/veyron2/naming"
"v.io/core/veyron2/services/mgmt/stats"
+ "v.io/core/veyron2/vdl"
"v.io/core/veyron2/vlog"
)
@@ -161,7 +163,14 @@
c <- ptuple
return
}
- pid := int(v.(int64))
+ // Convert the stat value from *vdl.Value into an int pid.
+ var pid int
+ if err := vdl.Convert(&pid, v); err != nil {
+ ptuple.err = fmt.Errorf("__debug/stats/system/pid isn't an integer: %v", err)
+ vlog.Errorf(ptuple.err.Error())
+ c <- ptuple
+ return
+ }
ptuple.pid = pid
// Update the instance info.
diff --git a/services/mgmt/device/impl/instance_reaping_test.go b/services/mgmt/device/impl/instance_reaping_test.go
index bd2a50e..17f376a 100644
--- a/services/mgmt/device/impl/instance_reaping_test.go
+++ b/services/mgmt/device/impl/instance_reaping_test.go
@@ -12,6 +12,7 @@
"v.io/core/veyron2/naming"
"v.io/core/veyron2/services/mgmt/application"
"v.io/core/veyron2/services/mgmt/stats"
+ "v.io/core/veyron2/vdl"
"v.io/core/veyron/lib/flags/consts"
"v.io/core/veyron/lib/modules"
@@ -79,9 +80,9 @@
if err != nil {
t.Fatalf("Value() failed: %v\n", err)
}
- pid, ok := v.(int64)
- if !ok {
- t.Fatalf("pid returned from stats interface is not an int")
+ var pid int
+ if err := vdl.Convert(&pid, v); err != nil {
+ t.Fatalf("pid returned from stats interface is not an int: %v", err)
}
verifyAppState(t, root, appID, instance1ID, "started")
@@ -111,7 +112,7 @@
if err != nil {
t.Fatalf("Value() failed: %v\n", err)
}
- return int(v.(int64))
+ return int(v.Int())
}
func TestReapReconciliation(t *testing.T) {
diff --git a/services/mgmt/device/impl/proxy_invoker.go b/services/mgmt/device/impl/proxy_invoker.go
index 340ca7c..96b7359 100644
--- a/services/mgmt/device/impl/proxy_invoker.go
+++ b/services/mgmt/device/impl/proxy_invoker.go
@@ -8,6 +8,7 @@
"v.io/core/veyron2/ipc"
"v.io/core/veyron2/naming"
"v.io/core/veyron2/services/security/access"
+ "v.io/core/veyron2/vdl"
"v.io/core/veyron2/vdl/vdlroot/src/signature"
)
@@ -36,13 +37,15 @@
var _ ipc.Invoker = (*proxyInvoker)(nil)
-func (p *proxyInvoker) Prepare(method string, numArgs int) (argptrs, tags []interface{}, err error) {
+func (p *proxyInvoker) Prepare(method string, numArgs int) (argptrs []interface{}, tags []*vdl.Value, _ error) {
+ // TODO(toddw): Change argptrs to be filled in with *vdl.Value, to avoid
+ // unnecessary type lookups.
argptrs = make([]interface{}, numArgs)
for i, _ := range argptrs {
var x interface{}
argptrs[i] = &x
}
- tags = []interface{}{p.access}
+ tags = []*vdl.Value{vdl.ValueOf(p.access)}
return
}
diff --git a/services/mgmt/repository/repository.vdl.go b/services/mgmt/repository/repository.vdl.go
index a460dca..b2aab89 100644
--- a/services/mgmt/repository/repository.vdl.go
+++ b/services/mgmt/repository/repository.vdl.go
@@ -205,7 +205,7 @@
{"Profiles", ``}, // []string
{"Envelope", ``}, // application.Envelope
},
- Tags: []vdl.AnyRep{access.Tag("Write")},
+ Tags: []*vdl.Value{vdl.ValueOf(access.Tag("Write"))},
},
{
Name: "Remove",
@@ -213,7 +213,7 @@
InArgs: []ipc.ArgDesc{
{"Profile", ``}, // string
},
- Tags: []vdl.AnyRep{access.Tag("Write")},
+ Tags: []*vdl.Value{vdl.ValueOf(access.Tag("Write"))},
},
},
}
@@ -395,7 +395,7 @@
OutArgs: []ipc.ArgDesc{
{"", ``}, // profile.Specification
},
- Tags: []vdl.AnyRep{access.Tag("Read")},
+ Tags: []*vdl.Value{vdl.ValueOf(access.Tag("Read"))},
},
{
Name: "Put",
@@ -403,12 +403,12 @@
InArgs: []ipc.ArgDesc{
{"Specification", ``}, // profile.Specification
},
- Tags: []vdl.AnyRep{access.Tag("Write")},
+ Tags: []*vdl.Value{vdl.ValueOf(access.Tag("Write"))},
},
{
Name: "Remove",
Doc: "// Remove removes the profile specification for the profile\n// identified through the object name suffix.",
- Tags: []vdl.AnyRep{access.Tag("Write")},
+ Tags: []*vdl.Value{vdl.ValueOf(access.Tag("Write"))},
},
},
}
diff --git a/services/mgmt/stats/impl/stats.go b/services/mgmt/stats/impl/stats.go
index 6585162..5df5f0d 100644
--- a/services/mgmt/stats/impl/stats.go
+++ b/services/mgmt/stats/impl/stats.go
@@ -3,6 +3,7 @@
package impl
import (
+ "reflect"
"time"
libstats "v.io/core/veyron/lib/stats"
@@ -69,7 +70,7 @@
c := watchtypes.Change{
Name: v.Key,
State: watchtypes.Exists,
- Value: v.Value,
+ Value: vdl.ValueOf(v.Value),
}
changes = append(changes, c)
}
@@ -94,18 +95,21 @@
}
// Value returns the value of the receiver object.
-func (i *statsService) Value(ctx ipc.ServerContext) (vdl.AnyRep, error) {
+func (i *statsService) Value(ctx ipc.ServerContext) (*vdl.Value, error) {
vlog.VI(1).Infof("%v.Value()", i.suffix)
- v, err := libstats.Value(i.suffix)
- switch err {
- case libstats.ErrNotFound:
+ rv, err := libstats.Value(i.suffix)
+ switch {
+ case err == libstats.ErrNotFound:
return nil, verror.New(verror.ErrNoExist, ctx.Context(), i.suffix)
- case libstats.ErrNoValue:
+ case err == libstats.ErrNoValue:
return nil, stats.NewErrNoValue(ctx.Context(), i.suffix)
- case nil:
- return v, nil
- default:
+ case err != nil:
return nil, verror.New(errOperationFailed, ctx.Context(), i.suffix)
}
+ vv, err := vdl.ValueFromReflect(reflect.ValueOf(rv))
+ if err != nil {
+ return nil, verror.New(verror.ErrInternal, ctx.Context(), i.suffix, err)
+ }
+ return vv, nil
}
diff --git a/services/mgmt/stats/impl/stats_test.go b/services/mgmt/stats/impl/stats_test.go
index fbc4fe9..f036846 100644
--- a/services/mgmt/stats/impl/stats_test.go
+++ b/services/mgmt/stats/impl/stats_test.go
@@ -12,6 +12,7 @@
"v.io/core/veyron2/security"
"v.io/core/veyron2/services/mgmt/stats"
"v.io/core/veyron2/services/watch/types"
+ "v.io/core/veyron2/vdl"
libstats "v.io/core/veyron/lib/stats"
"v.io/core/veyron/lib/stats/histogram"
@@ -106,7 +107,7 @@
t.Fatalf("expected more stream values")
}
got := iterator.Value()
- expected := types.Change{Name: "testing/foo/bar", Value: int64(10), ResumeMarker: noRM}
+ expected := types.Change{Name: "testing/foo/bar", Value: vdl.Int64Value(10), ResumeMarker: noRM}
if !reflect.DeepEqual(got, expected) {
t.Errorf("unexpected result. Got %#v, want %#v", got, expected)
}
@@ -117,7 +118,7 @@
t.Fatalf("expected more stream values")
}
got = iterator.Value()
- expected = types.Change{Name: "testing/foo/bar", Value: int64(15), ResumeMarker: noRM}
+ expected = types.Change{Name: "testing/foo/bar", Value: vdl.Int64Value(15), ResumeMarker: noRM}
if !reflect.DeepEqual(got, expected) {
t.Errorf("unexpected result. Got %#v, want %#v", got, expected)
}
@@ -128,7 +129,7 @@
t.Fatalf("expected more stream values")
}
got = iterator.Value()
- expected = types.Change{Name: "testing/foo/bar", Value: int64(17), ResumeMarker: noRM}
+ expected = types.Change{Name: "testing/foo/bar", Value: vdl.Int64Value(17), ResumeMarker: noRM}
if !reflect.DeepEqual(got, expected) {
t.Errorf("unexpected result. Got %#v, want %#v", got, expected)
}
@@ -146,8 +147,8 @@
if err != nil {
t.Errorf("unexpected error: %v", err)
}
- if expected := int64(17); value != expected {
- t.Errorf("unexpected result. Got %v, want %v", value, expected)
+ if want := vdl.Int64Value(17); !vdl.EqualValue(value, want) {
+ t.Errorf("unexpected result. Got %v, want %v", value, want)
}
}
@@ -158,7 +159,7 @@
if err != nil {
t.Errorf("unexpected error: %v", err)
}
- want := istats.HistogramValue{
+ want := vdl.ValueOf(istats.HistogramValue{
Count: 10,
Sum: 45,
Min: 0,
@@ -170,9 +171,9 @@
istats.HistogramBucket{LowBound: 7, Count: 3},
istats.HistogramBucket{LowBound: 15, Count: 0},
},
- }
- if !reflect.DeepEqual(value, want) {
- t.Errorf("unexpected result. Got %#v, want %#v", value, want)
+ })
+ if !vdl.EqualValue(value, want) {
+ t.Errorf("unexpected result. Got %v, want %v", value, want)
}
}
}