Skip to content

Commit

Permalink
internal/core/adt: catch if OpContext.Version is unset
Browse files Browse the repository at this point in the history
If the evaluator is doing work without knowing its own version,
that means that somewhere we forgot to set the version correctly.

One such bug was SharedRuntime, which didn't use the Runtime constructor
and so left Version as zero even if CUE_EXPERIMENT=evalv3 was set.

Another was TestScheduler, which created an OpContext directly.
This used the default evaluator version when it was the zero value,
so restore that behavior now that we must specify the default by name.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I9e926061c1bf00e8851966edb3ee1add662b5884
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202745
Reviewed-by: Marcel van Lohuizen <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
mvdan committed Oct 25, 2024
1 parent f17356d commit 52c4418
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 4 deletions.
5 changes: 4 additions & 1 deletion internal/core/adt/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,11 @@ func New(v *Vertex, cfg *Config) *OpContext {
return ctx
}

// See also: unreachableForDev
// See also: [unreachableForDev]
func (c *OpContext) isDevVersion() bool {
if c.Version == internal.EvalVersionUnset {
panic("OpContext was not provided with an evaluator version")
}
return c.Version == internal.DevVersion
}

Expand Down
2 changes: 2 additions & 0 deletions internal/core/adt/sched_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"
"testing"

"cuelang.org/go/internal"
"cuelang.org/go/internal/cuetest"
)

Expand All @@ -35,6 +36,7 @@ const (
// TestScheduler tests the non-CUE specific scheduler functionality.
func TestScheduler(t *testing.T) {
ctx := &OpContext{
Version: internal.DefaultVersion,
taskContext: taskContext{
counterMask: c1 | c2 | c3 | c4,
complete: func(s *scheduler) condition { return 0 },
Expand Down
26 changes: 25 additions & 1 deletion internal/core/runtime/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (

"cuelang.org/go/cue/build"
"cuelang.org/go/cue/errors"
"cuelang.org/go/internal"
"cuelang.org/go/internal/core/adt"
"cuelang.org/go/internal/cueexperiment"
)

type PackageFunc func(ctx adt.Runtime) (*adt.Vertex, errors.Error)
Expand All @@ -42,7 +44,29 @@ func (x *index) RegisterBuiltin(importPath string, f PackageFunc) {
x.builtinShort[base] = importPath
}

var SharedRuntime = &Runtime{index: sharedIndex}
// We use a sync.OnceValue below so that cueexperiment.Init is only called
// the first time that the API is used, letting the user set $CUE_EXPERIMENT globally
// as part of their package init if they want to.
var SharedRuntime = sync.OnceValue(func() *Runtime {
r := &Runtime{index: sharedIndex}
// The version logic below is copied from [Runtime.Init];
// consider refactoring to share the code if it gets any more complicated.
//
// TODO(mvdan,mpvl): Note that SharedRuntime follows the globally set evaluator version,
// which may be different than what was supplied via Go code for each context like
// via cuecontext.EvaluatorVersion(cuecontext.EvalV3).
// This does not cause issues between evalv2 and evalv3 as they use the same ADT,
// but future evaluator versions may not be compatible at that level.
// We should consider using one SharedRuntime per evaluator version,
// or getting rid of SharedRuntime altogether.
cueexperiment.Init()
if cueexperiment.Flags.EvalV3 {
r.version = internal.DevVersion
} else {
r.version = internal.DefaultVersion
}
return r
})

// BuiltinPackagePath converts a short-form builtin package identifier to its
// full path or "" if this doesn't exist.
Expand Down
3 changes: 2 additions & 1 deletion internal/core/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func (r *Runtime) BuildData(b *build.Instance) (x interface{}, ok bool) {
return x, ok
}

// New is a wrapper for NewVersioned(internal.DefaultVersion).
// New is short for [NewWithSettings] while obeying `CUE_EXPERIMENT=evalv3`
// for the evaluator version and using zero [cuedebug] flags.
func New() *Runtime {
r := &Runtime{}
r.Init()
Expand Down
2 changes: 1 addition & 1 deletion internal/value/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func Make(ctx *adt.OpContext, v adt.Value) cue.Value {
// UnifyBuiltin returns the given Value unified with the given builtin template.
func UnifyBuiltin(v cue.Value, kind string) cue.Value {
pkg, name, _ := strings.Cut(kind, ".")
s := runtime.SharedRuntime.LoadImport(pkg)
s := runtime.SharedRuntime().LoadImport(pkg)
if s == nil {
return v
}
Expand Down

0 comments on commit 52c4418

Please sign in to comment.