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 Dispatch-Trailer: {"type":"trybot","CL":1206291,"patchset":1,"ref":"refs/changes/91/1206291/1","targetBranch":"master"}
cue-lang · Dec 23, 2024 · c9dffa3 · c9dffa3
1 parent e2717cd
commit c9dffa3
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 97 deletions.
diff --git a/cue/testdata/disjunctions/errors.txtar b/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:

diff --git a/internal/core/adt/unify.go b/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 {