From c1bf55049d80ba48245a276bc8121ea1eea19fc8 Mon Sep 17 00:00:00 2001
From: Marcel van Lohuizen <mpvl@golang.org>
Date: Fri, 14 Jan 2022 10:10:53 +0100
Subject: [PATCH] internal/core/eval: dereference indirections earlier

Some temporary fields, like comprehension bindings, add
an indirection to Vertices. Bugs may arise if these are not
properly unwrapped.

Lookup now derferences such indirections. Unwrap does
the same as a second line of defense.

Signed-off-by: Marcel van Lohuizen <mpvl@golang.org>

Change-Id: Ia7832ab48ecebbb8c3c9341f99203859a7418e2a
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/531093
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>
---
 cue/testdata/comprehensions/fields.txtar | 91 +++++++++++++++---------
 internal/core/adt/composite.go           | 16 ++++-
 internal/core/adt/context.go             | 11 ++-
 3 files changed, 77 insertions(+), 41 deletions(-)

diff --git a/cue/testdata/comprehensions/fields.txtar b/cue/testdata/comprehensions/fields.txtar
index 1974d3cfdba..641663d2881 100644
--- a/cue/testdata/comprehensions/fields.txtar
+++ b/cue/testdata/comprehensions/fields.txtar
@@ -6,53 +6,78 @@ module: "example.com"
 -- in.cue --
 import "strings"
 
-#User: {
-    tags_str: string
-    tags_map: {
-         for k, v in strings.Split(tags_str, " ") {
-             "\(v)": string
-         }
-         "{a}": string
+dynamic: {
+    for _, s in ["foo"] {
+        (s):       1
+        "\(s)bar": 2
     }
 }
 
-user: {
-    #User
-    tags_str: "b {c}"
+issue560: {
+    #User: {
+        tags_str: string
+        tags_map: {
+            for k, v in strings.Split(tags_str, " ") {
+                "\(v)": string
+            }
+            "{a}": string
+        }
+    }
+
+    user: {
+        #User
+        tags_str: "b {c}"
+    }
 }
 -- out/eval --
 (struct){
-  #User: (#struct){
-    tags_str: (string){ string }
-    tags_map: (_|_){
-      // [incomplete] error in call to strings.Split: non-concrete value string:
-      //     ./in.cue:6:22
-      "{a}": (string){ string }
-    }
+  dynamic: (struct){
+    foo: (int){ 1 }
+    foobar: (int){ 2 }
   }
-  user: (#struct){
-    tags_str: (string){ "b {c}" }
-    tags_map: (#struct){
-      "{a}": (string){ string }
-      b: (string){ string }
-      "{c}": (string){ string }
+  issue560: (struct){
+    #User: (#struct){
+      tags_str: (string){ string }
+      tags_map: (_|_){
+        // [incomplete] error in call to strings.Split: non-concrete value string:
+        //     ./in.cue:14:25
+        "{a}": (string){ string }
+      }
+    }
+    user: (#struct){
+      tags_str: (string){ "b {c}" }
+      tags_map: (#struct){
+        "{a}": (string){ string }
+        b: (string){ string }
+        "{c}": (string){ string }
+      }
     }
   }
 }
 -- out/compile --
 --- in.cue
 {
-  #User: {
-    tags_str: string
-    tags_map: {
-      for k, v in 〈import;strings〉.Split(〈1;tags_str〉, " ") {
-        "\(〈1;v〉)": string
-      }
-      "{a}": string
+  dynamic: {
+    for _, s in [
+      "foo",
+    ] {
+      〈1;s〉: 1
+      "\(〈1;s〉)bar": 2
     }
   }
-  user: {
-    〈1;#User〉
-    tags_str: "b {c}"
+  issue560: {
+    #User: {
+      tags_str: string
+      tags_map: {
+        for k, v in 〈import;strings〉.Split(〈1;tags_str〉, " ") {
+          "\(〈1;v〉)": string
+        }
+        "{a}": string
+      }
+    }
+    user: {
+      〈1;#User〉
+      tags_str: "b {c}"
+    }
   }
 }
diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
index 4551d778db7..bf1e24eb5b8 100644
--- a/internal/core/adt/composite.go
+++ b/internal/core/adt/composite.go
@@ -487,7 +487,7 @@ func Unwrap(v Value) Value {
 	if !ok {
 		return v
 	}
-	// b, _ := x.BaseValue.(*Bottom)
+	x = x.Indirect()
 	if n := x.state; n != nil && isCyclePlaceholder(x.BaseValue) {
 		if n.errs != nil && !n.errs.IsIncomplete() {
 			return n.errs
@@ -499,6 +499,19 @@ func Unwrap(v Value) Value {
 	return x.Value()
 }
 
+// Indirect unrolls indirections of Vertex values. These may be introduced,
+// for instance, by temporary bindings such as comprehension values.
+// It returns v itself if v does not point to another Vertex.
+func (v *Vertex) Indirect() *Vertex {
+	for {
+		arc, ok := v.BaseValue.(*Vertex)
+		if !ok {
+			return v
+		}
+		v = arc
+	}
+}
+
 // OptionalType is a bit field of the type of optional constraints in use by an
 // Acceptor.
 type OptionalType int8
@@ -639,6 +652,7 @@ func (v *Vertex) IsList() bool {
 func (v *Vertex) Lookup(f Feature) *Vertex {
 	for _, a := range v.Arcs {
 		if a.Label == f {
+			a = a.Indirect()
 			return a
 		}
 	}
diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go
index 364068cbc08..0c4bb61a5e8 100644
--- a/internal/core/adt/context.go
+++ b/internal/core/adt/context.go
@@ -400,13 +400,7 @@ func (c *OpContext) Resolve(env *Environment, r Resolver) (*Vertex, *Bottom) {
 		return nil, arc.ChildErrors
 	}
 
-	for {
-		x, ok := arc.BaseValue.(*Vertex)
-		if !ok {
-			break
-		}
-		arc = x
-	}
+	arc = arc.Indirect()
 
 	return arc, err
 }
@@ -761,6 +755,9 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, state VertexStat
 	}
 
 	a := x.Lookup(l)
+	if a != nil {
+		a = a.Indirect()
+	}
 
 	var hasCycle bool
 outer: