Skip to content

Commit

Permalink
internal/core/adt: add Sharing option in CUE_DEBUG
Browse files Browse the repository at this point in the history
This allows users to disable structure sharing.

Also:
- use cuedebug.Config in tests
- keep a copy of cuedebug.Config to runtime.Runtime

We modified existing methods for the runtime types,
rather than adding a new one, to enforce that we set
both values on all call sites: typically both should be
set.

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I7d2ea091cba0d79ba7ddbeedae67ee2487a7b95c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193637
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
mpvl committed Apr 23, 2024
1 parent 891a8a0 commit d900867
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cmd/cue/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
// in a non-tooling context.
if cueexperiment.Flags.EvalV3 {
const dev = internal.DevVersion
(*cueruntime.Runtime)(c.ctx).SetVersion(internal.EvaluatorVersion(dev))
(*cueruntime.Runtime)(c.ctx).SetSettings(internal.EvaluatorVersion(dev), cuedebug.Flags)
}

err := f(c, args)
Expand Down
6 changes: 4 additions & 2 deletions internal/core/adt/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ type Runtime interface {
// type if available.
LoadType(t reflect.Type) (src ast.Expr, expr Expr, ok bool)

EvaluatorVersion() internal.EvaluatorVersion
Settings() (internal.EvaluatorVersion, cuedebug.Config)
}

type Config struct {
Expand All @@ -180,11 +180,13 @@ func New(v *Vertex, cfg *Config) *OpContext {
if cfg.Runtime == nil {
panic("nil Runtime")
}
version, flags := cfg.Runtime.Settings()
ctx := &OpContext{
Runtime: cfg.Runtime,
Format: cfg.Format,
vertex: v,
Version: cfg.Runtime.EvaluatorVersion(),
Version: version,
Config: flags,
taskContext: schedConfig,
}
if v != nil {
Expand Down
26 changes: 17 additions & 9 deletions internal/core/adt/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"cuelang.org/go/internal/core/eval"
"cuelang.org/go/internal/core/runtime"
"cuelang.org/go/internal/core/validate"
"cuelang.org/go/internal/cuedebug"
"cuelang.org/go/internal/cuetxtar"
_ "cuelang.org/go/pkg"
)
Expand All @@ -58,7 +59,7 @@ func TestEval(t *testing.T) {
}

test.Run(t, func(tc *cuetxtar.Test) {
runEvalTest(tc, internal.DefaultVersion)
runEvalTest(tc, internal.DefaultVersion, cuedebug.Config{})
})
}

Expand All @@ -79,6 +80,10 @@ func TestEvalAlpha(t *testing.T) {
//
// adt.DebugDeps = true // check unmatched dependencies.

flags := cuedebug.Config{
Sharing: true,
}

var todoAlpha = map[string]string{
// Crashes and hangs
"cycle/evaluate": "hang",
Expand Down Expand Up @@ -111,7 +116,7 @@ func TestEvalAlpha(t *testing.T) {
}
ran++

errorCount += runEvalTest(t, internal.DevVersion)
errorCount += runEvalTest(t, internal.DevVersion, flags)
})

t.Logf("todo: %d, ran: %d, skipped: %d, nodeErrors: %d",
Expand Down Expand Up @@ -140,9 +145,9 @@ func skipFiles(a ...*ast.File) (reason string) {
return reason
}

func runEvalTest(t *cuetxtar.Test, version internal.EvaluatorVersion) (errorCount int) {
func runEvalTest(t *cuetxtar.Test, version internal.EvaluatorVersion, flags cuedebug.Config) (errorCount int) {
a := t.Instance()
r := runtime.NewVersioned(version)
r := runtime.NewWithSettings(version, flags)

v, err := r.Build(nil, a)
if err != nil {
Expand All @@ -153,6 +158,7 @@ func runEvalTest(t *cuetxtar.Test, version internal.EvaluatorVersion) (errorCoun
e := eval.New(r)
ctx := e.NewContext(v)
ctx.Version = version
ctx.Config = flags
v.Finalize(ctx)

// Print discrepancies in dependencies.
Expand Down Expand Up @@ -227,12 +233,14 @@ func runEvalTest(t *cuetxtar.Test, version internal.EvaluatorVersion) (errorCoun

// TestX is for debugging. Do not delete.
func TestX(t *testing.T) {
var verbosity int
verbosity = 1 // comment to turn logging off.

adt.DebugDeps = true
// adt.OpenGraphs = true

flags := cuedebug.Config{
Sharing: true, // Uncomment to turn sharing off.
LogEval: 1, // Uncomment to turn logging off
}

var version internal.EvaluatorVersion
version = internal.DevVersion // comment to use default implementation.

Expand All @@ -253,7 +261,7 @@ module: "mod.test"
t.Fatal(instance.Err)
}

r := runtime.NewVersioned(version)
r := runtime.NewWithSettings(version, flags)

v, err := r.Build(nil, instance)
if err != nil {
Expand All @@ -262,7 +270,7 @@ module: "mod.test"

e := eval.New(r)
ctx := e.NewContext(v)
ctx.LogEval = verbosity
ctx.Config = flags
v.Finalize(ctx)

out := debug.NodeString(r, v, nil)
Expand Down
3 changes: 3 additions & 0 deletions internal/core/adt/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) {
func (n *nodeContext) shareIfPossible(c Conjunct, arc *Vertex, id CloseInfo) bool {
// TODO: have an experiment here to enable or disable structure sharing.
// return false
if !n.ctx.Sharing {
return false
}

if n.noSharing || n.isShared || n.ctx.errs != nil {
return false
Expand Down
5 changes: 4 additions & 1 deletion internal/core/eval/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ func Evaluate(r adt.Runtime, v *adt.Vertex) {
}

func New(r adt.Runtime) *Unifier {
return &Unifier{r: r, e: NewContext(r, nil)}
ctx := NewContext(r, nil)
// TODO: we could access these directly if we can use runtime.Runtime directly.
ctx.Version, ctx.Config = r.Settings()
return &Unifier{r: r, e: ctx}
}

type Unifier struct {
Expand Down
21 changes: 13 additions & 8 deletions internal/core/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package runtime
import (
"cuelang.org/go/cue/build"
"cuelang.org/go/internal"
"cuelang.org/go/internal/cuedebug"
)

// A Runtime maintains data structures for indexing and reuse for evaluation.
Expand All @@ -30,10 +31,12 @@ type Runtime struct {
interpreters map[string]Interpreter

version internal.EvaluatorVersion

flags cuedebug.Config
}

func (r *Runtime) EvaluatorVersion() internal.EvaluatorVersion {
return r.version
func (r *Runtime) Settings() (internal.EvaluatorVersion, cuedebug.Config) {
return r.version, r.flags
}

func (r *Runtime) SetBuildData(b *build.Instance, x interface{}) {
Expand All @@ -52,18 +55,20 @@ func New() *Runtime {
return r
}

// NewVersioned creates a new Runtime using the given runtime version.
// The builtins registered with RegisterBuiltin are available for evaluation.
func NewVersioned(v internal.EvaluatorVersion) *Runtime {
r := &Runtime{version: v}
// NewWithSettings creates a new Runtime using the given runtime version and
// debug flags. The builtins registered with RegisterBuiltin are available for
// evaluation.
func NewWithSettings(v internal.EvaluatorVersion, flags cuedebug.Config) *Runtime {
r := &Runtime{version: v, flags: flags}
r.Init()
return r
}

// SetVersion sets the version to use for the Runtime. This should only be set
// SetSettings sets the version to use for the Runtime. This should only be set
// before first use.
func (r *Runtime) SetVersion(v internal.EvaluatorVersion) {
func (r *Runtime) SetSettings(v internal.EvaluatorVersion, flags cuedebug.Config) {
r.version = v
r.flags = flags
}

// IsInitialized reports whether the runtime has been initialized.
Expand Down
8 changes: 8 additions & 0 deletions internal/cuedebug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ var Flags Config
type Config struct {
HTTP bool

// TODO: consider moving these evaluator-related options into a separate
// struct, so that it can be used in an API. We should use embedding,
// or some other mechanism, in that case to allow for the full set of
// allowed environment variables to be known.

// Strict sets whether extra aggressive checking should be done.
// This should typically default to true for pre-releases and default to
// false otherwise.
Expand All @@ -23,6 +28,9 @@ type Config struct {
// 0: no logging
// 1: logging
LogEval int

// Sharing enables structure sharing.
Sharing bool `envflag:"default:true"`
}

// Init initializes Flags. Note: this isn't named "init" because we
Expand Down

0 comments on commit d900867

Please sign in to comment.