internal/core/adt: propagate child errors

In V3, because closedness is computed out of
band with the processing of an arc, it may be
that an arc gets an error _after_ it has been
processed. For this reasons, we propagate it in
a separate loop after all other arcs have been
processed.

To avoid spurious errors, we now need to explicitly
check for an arc error before calling
reportFieldMismatch. Theoretically it is possible
that there is still a spurious error. This only occurs,
however, if the arc is already erroneous, so this is
not a huge deal.

Fixes #3576

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Idfec48db39be56155c3376b4284bd67a4e8bef20
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1206291
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

cue/testdata/disjunctions/errors.txtar

@@ -66,7 +66,7 @@ issue3576: full: {
 			optionsValue: options
 		}
 	}
-	
+
 	#Copy: {
 		copy!: string
 		options?: #Option | [#Option, ...]
@@ -78,7 +78,7 @@ issue3576: full: {
 	#Option: {}
 
 	foo: #Run | #Copy
-	foo: run: "make"	
+	foo: run: "make"
 }
 -- out/eval/stats --
 Leaks:  0
@@ -180,15 +180,9 @@ Result:
       }
       #B: (#struct){
       }
-      foo: (struct){ |((#struct){
-          a: (_|_){
-            // [eval] issue3576.reduced.foo.a: field not allowed:
-            //     ./issue3576.cue:5:11
-            //     ./issue3576.cue:9:7
-          }
-        }, (#struct){
-          a: (string){ "1" }
-        }) }
+      foo: (#struct){
+        a: (string){ "1" }
+      }
     }
     full: (struct){
       #Run: (#struct){
@@ -209,26 +203,14 @@ Result:
       }
       #Option: (#struct){
       }
-      foo: (struct){ |((#struct){
-          run: (string){ "make" }
-          options?: ((list|struct)){ |((#struct){
-            }, (list){
-              0: (#struct){
-              }
-            }) }
-        }, (#struct){
-          run: (_|_){
-            // [eval] issue3576.full.foo.run: field not allowed:
-            //     ./issue3576.cue:24:18
-            //     ./issue3576.cue:31:7
-          }
-          copy!: (string){ string }
-          options?: ((list|struct)){ |((#struct){
-            }, (list){
-              0: (#struct){
-              }
-            }) }
-        }) }
+      foo: (#struct){
+        run: (string){ "make" }
+        options?: ((list|struct)){ |((#struct){
+          }, (list){
+            0: (#struct){
+            }
+          }) }
+      }
     }
   }
   issue3581: (struct){
@@ -329,61 +311,7 @@ diff old new
    }
    issue3576: (struct){
      reduced: (struct){
-@@ -99,9 +87,15 @@
-       }
-       #B: (#struct){
-       }
--      foo: (#struct){
--        a: (string){ "1" }
--      }
-+      foo: (struct){ |((#struct){
-+          a: (_|_){
-+            // [eval] issue3576.reduced.foo.a: field not allowed:
-+            //     ./issue3576.cue:5:11
-+            //     ./issue3576.cue:9:7
-+          }
-+        }, (#struct){
-+          a: (string){ "1" }
-+        }) }
-     }
-     full: (struct){
-       #Run: (#struct){
-@@ -122,14 +116,26 @@
-       }
-       #Option: (#struct){
-       }
--      foo: (#struct){
--        run: (string){ "make" }
--        options?: ((list|struct)){ |((#struct){
--          }, (list){
--            0: (#struct){
--            }
--          }) }
--      }
-+      foo: (struct){ |((#struct){
-+          run: (string){ "make" }
-+          options?: ((list|struct)){ |((#struct){
-+            }, (list){
-+              0: (#struct){
-+              }
-+            }) }
-+        }, (#struct){
-+          run: (_|_){
-+            // [eval] issue3576.full.foo.run: field not allowed:
-+            //     ./issue3576.cue:24:18
-+            //     ./issue3576.cue:31:7
-+          }
-+          copy!: (string){ string }
-+          options?: ((list|struct)){ |((#struct){
-+            }, (list){
-+              0: (#struct){
-+              }
-+            }) }
-+        }) }
-     }
-   }
-   issue3581: (struct){
-@@ -136,13 +142,11 @@
+@@ -136,13 +124,11 @@
      reduced: (struct){
        list: (_|_){
          // [incomplete] issue3581.reduced.list: 2 errors in empty disjunction:

internal/core/adt/unify.go

@@ -565,21 +565,11 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
 			continue
 		}
 
-		// Errors are allowed in let fields. Handle errors and failure to
-		// complete accordingly.
-		if !a.Label.IsLet() && a.ArcType <= ArcRequired {
-			a := a.DerefValue()
-			if err := a.Bottom(); err != nil {
-				n.AddChildError(err)
-			}
-			success = true // other arcs are irrelevant
-		}
-
 		// TODO: harmonize this error with "cannot combine"
 		switch {
 		case a.ArcType > ArcRequired, !a.Label.IsString():
 		case n.kind&StructKind == 0:
-			if !n.node.IsErr() {
+			if !n.node.IsErr() && !a.IsErr() {
 				n.reportFieldMismatch(pos(a.Value()), nil, a.Label, n.node.Value())
 			}
 			// case !wasVoid:
@@ -610,6 +600,18 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
 	}
 	n.node.Arcs = n.node.Arcs[:k]
 
+	for _, a := range n.node.Arcs {
+		// Errors are allowed in let fields. Handle errors and failure to
+		// complete accordingly.
+		if !a.Label.IsLet() && a.ArcType <= ArcRequired {
+			a := a.DerefValue()
+			if err := a.Bottom(); err != nil {
+				n.AddChildError(err)
+			}
+			success = true // other arcs are irrelevant
+		}
+	}
+
 	// TODO: perhaps this code can go once we have builtins for comparing to
 	// bottom.
 	for _, c := range n.postChecks {