internal/core/adt: move cycle state saving logic to Expr

The and builtin uses EvaluateKeepState to weave
cycle information into further evaluation. This should
generally be done if Expr is used. We therefore move that
logic to Expr directly, so that this complicated piece
of code does not proliferate.

Issue #3649

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ib9460b0cf8e09065dc3a9b5a42f9f349c03d6fdc

internal/core/adt/call.go

@@ -69,18 +69,26 @@ func (c *CallContext) Args() []Value {
 	return c.args
 }
 
-// Expr return the nth argument expression. The value remains unevaluated.
-// If the call context represents a validator call, the argument will be offset
-// by 1.
-func (c *CallContext) Expr(i int) Expr {
+// Expr return the nth argument expression. The value is evaluated and any
+// cycle information is accumulated in the context. This allows cycles in
+// arguments to be detected.
+//
+// This method of getting an argument should be used when the argument is used
+// as a schema and may contain cycles.
+func (c *CallContext) Expr(i int) Value {
+	// If the call context represents a validator call, the argument will be
+	// offset by 1.
 	if c.isValidator {
 		if i == 0 {
 			c.Errf("Expr may not be called for 0th argument of validator")
 			return nil
 		}
 		i--
 	}
-	return c.call.Args[i]
+	x := c.call.Args[i]
+
+	// Evaluated while keeping any cycle information in the context.
+	return c.ctx.EvaluateKeepState(x)
 }
 
 func (c *CallContext) Errf(format string, args ...interface{}) *Bottom {

internal/core/adt/context.go

@@ -563,7 +563,7 @@ func (c *OpContext) Evaluate(env *Environment, x Expr) (result Value, complete b
 	return val, true
 }
 
-// EvaluateKeepState does an evaluate, but leaves any errors an cycle info
+// EvaluateKeepState does an evaluate, but leaves any errors and cycle info
 // within the context.
 func (c *OpContext) EvaluateKeepState(x Expr) (result Value) {
 	src := c.src

internal/core/compile/builtin.go

@@ -120,9 +120,7 @@ var andBuiltin = &adt.Builtin{
 		c := call.OpContext()
 		arg := call.Expr(0)
 
-		// Pass through the cycle information from evaluating the first argument.
-		v := c.EvaluateKeepState(arg)
-		list := c.RawElems(v)
+		list := c.RawElems(arg)
 		if len(list) == 0 {
 			return &adt.Top{}
 		}