From 826bd7bd782889a15bb8d1016d89cbaba2210855 Mon Sep 17 00:00:00 2001
From: Marcel van Lohuizen <mpvl@golang.org>
Date: Sat, 16 Jan 2021 17:26:00 +0100
Subject: [PATCH] internal/core/adt: fix regression in close builtin

Would in some cases recursively close.

Now uses just one method for marking closedness. Marking
Closed in the Vertex was only used so that the debug printer
would show the closed status. This has now fixed. This was
not possible before but is now as the code was recently
simplified.

Fixes #642

Change-Id: I9aab6a02f36ddbc8ed9bce356a2f4ad77cd30cda
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8228
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
---
 cue/testdata/builtins/closed.txtar | 97 +++++++++++++++++++++++++++++-
 internal/core/adt/closed.go        |  3 +
 internal/core/adt/eval.go          |  1 -
 internal/core/adt/optional.go      |  9 ++-
 4 files changed, 103 insertions(+), 7 deletions(-)

diff --git a/cue/testdata/builtins/closed.txtar b/cue/testdata/builtins/closed.txtar
index badd5f16b..501576eab 100644
--- a/cue/testdata/builtins/closed.txtar
+++ b/cue/testdata/builtins/closed.txtar
@@ -18,6 +18,30 @@ inDisjunctions: {
     x: syslog: xxx: {}
 }
 
+issue642: {
+    test: close({
+        a: _
+        b: x: _
+    } & {
+        [string]: y: _
+    })
+
+    test: a: x: _
+    test: b: x: _
+}
+
+// Issue 642
+withSubfields: {
+    test: close({
+        a: _
+        b: x: _
+        [string]: y: _
+    })
+
+    test: a: x: _
+    test: b: x: _
+}
+
 -- out/eval --
 Errors:
 b: field `x` not allowed:
@@ -29,13 +53,13 @@ Result:
 (_|_){
   // [eval]
   a: (#struct){
-    a: (#struct){
+    a: (struct){
       b: (int){ int }
     }
   }
   b: (_|_){
     // [eval]
-    a: (#struct){
+    a: (struct){
       b: (int){ int }
     }
     x: (_|_){
@@ -46,7 +70,7 @@ Result:
     }
   }
   c: (#struct){
-    a: (#struct){
+    a: (struct){
       b: (int){ int }
       c: (int){ int }
     }
@@ -118,6 +142,30 @@ Result:
     #Def: (#struct){
     }
   }
+  issue642: (struct){
+    test: (#struct){
+      a: (struct){
+        y: (_){ _ }
+        x: (_){ _ }
+      }
+      b: (struct){
+        x: (_){ _ }
+        y: (_){ _ }
+      }
+    }
+  }
+  withSubfields: (struct){
+    test: (#struct){
+      a: (struct){
+        y: (_){ _ }
+        x: (_){ _ }
+      }
+      b: (struct){
+        x: (_){ _ }
+        y: (_){ _ }
+      }
+    }
+  }
 }
 -- out/compile --
 --- in.cue
@@ -173,4 +221,47 @@ Result:
       }
     }
   }
+  issue642: {
+    test: close(({
+      a: _
+      b: {
+        x: _
+      }
+    } & {
+      [string]: {
+        y: _
+      }
+    }))
+    test: {
+      a: {
+        x: _
+      }
+    }
+    test: {
+      b: {
+        x: _
+      }
+    }
+  }
+  withSubfields: {
+    test: close({
+      a: _
+      b: {
+        x: _
+      }
+      [string]: {
+        y: _
+      }
+    })
+    test: {
+      a: {
+        x: _
+      }
+    }
+    test: {
+      b: {
+        x: _
+      }
+    }
+  }
 }
diff --git a/internal/core/adt/closed.go b/internal/core/adt/closed.go
index f68bc61be..c38596749 100644
--- a/internal/core/adt/closed.go
+++ b/internal/core/adt/closed.go
@@ -227,6 +227,9 @@ func (c *closeInfo) isClosed() bool {
 
 func isClosed(v *Vertex) bool {
 	for _, s := range v.Structs {
+		if s.IsClosed {
+			return true
+		}
 		for c := s.closeInfo; c != nil; c = c.parent {
 			if c.isClosed() {
 				return true
diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
index 4c4b5099d..3053294c6 100644
--- a/internal/core/adt/eval.go
+++ b/internal/core/adt/eval.go
@@ -1299,7 +1299,6 @@ func (n *nodeContext) addValueConjunct(env *Environment, v Value, id CloseInfo)
 			n.aStruct = x
 			n.aStructID = id
 			if m.NeedClose {
-				n.node.Closed = true // TODO: remove.
 				id = id.SpawnRef(x, IsDef(x), x)
 				id.IsClosed = true
 			}
diff --git a/internal/core/adt/optional.go b/internal/core/adt/optional.go
index 81c73c340..504d0f679 100644
--- a/internal/core/adt/optional.go
+++ b/internal/core/adt/optional.go
@@ -20,13 +20,16 @@ package adt
 func (o *StructInfo) MatchAndInsert(c *OpContext, arc *Vertex) {
 	env := o.Env
 
+	closeInfo := o.CloseInfo
+	closeInfo.IsClosed = false
+
 	// Match normal fields
 	matched := false
 outer:
 	for _, f := range o.Fields {
 		if f.Label == arc.Label {
 			for _, e := range f.Optional {
-				arc.AddConjunct(MakeConjunct(env, e, o.CloseInfo))
+				arc.AddConjunct(MakeConjunct(env, e, closeInfo))
 			}
 			matched = true
 			break outer
@@ -49,7 +52,7 @@ outer:
 		// }
 		if matchBulk(c, env, b, arc.Label) {
 			matched = true
-			arc.AddConjunct(MakeConjunct(&bulkEnv, b, o.CloseInfo))
+			arc.AddConjunct(MakeConjunct(&bulkEnv, b, closeInfo))
 		}
 	}
 	if matched {
@@ -62,7 +65,7 @@ outer:
 
 	// match others
 	for _, x := range o.Additional {
-		arc.AddConjunct(MakeConjunct(&addEnv, x, o.CloseInfo))
+		arc.AddConjunct(MakeConjunct(&addEnv, x, closeInfo))
 	}
 }