cue/errors: avoid simple duplication when gathering errors

The (benchmark) test case added caused immense duplication of errors
(over 43 million). The cause is unclear, but it's suspected a bug in
comprehensions. The solution here is to detect, in errors.Append, that
the two errors are the same, and if so, avoid creating a list of both.

Fixes #3307

Signed-off-by: Matthew Sackman <[email protected]>
Change-Id: I1cb3baf00888072c5a4811f9acc95fac9ed49d4b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199401
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: cuematthew

cue/errors/errors.go

@@ -274,16 +274,22 @@ func (e *posError) Position() token.Pos         { return e.pos }
 
 // Append combines two errors, flattening Lists as necessary.
 func Append(a, b Error) Error {
-	switch x := a.(type) {
+	switch a := a.(type) {
 	case nil:
 		return b
 	case list:
-		return appendToList(x, b)
+		return appendToList(a, b)
 	}
-	// Preserve order of errors.
-	list := appendToList(nil, a)
-	list = appendToList(list, b)
-	return list
+	switch b := b.(type) {
+	case nil:
+		return a
+	case list:
+		return appendToList(list{a}, b)
+	}
+	if a == b {
+		return a
+	}
+	return list{a, b}
 }
 
 // Errors reports the individual errors associated with an error, which is

cue/testdata/benchmarks/issue3307.txtar

@@ -0,0 +1,69 @@
+-- in.cue --
+if _request.enabled {
+    r1: {
+        k1: 0
+        k2: 0
+        k3: 0
+        k4: 0
+        k5: 0
+        k6: 0
+        k7: 0
+        k8: 0
+        k9: 0
+        k10: 0
+        k11: 0
+        k12: 0
+        k13: 0
+        k14: 0
+        k15: 0
+        k16: 0
+        k17: 0
+    }
+}
+
+_request: enabled: _
+-- out/eval/stats --
+Leaks:  0
+Freed:  21
+Reused: 16
+Allocs: 5
+Retain: 72
+
+Unifications: 21
+Conjuncts:    3
+Disjuncts:    93
+-- out/eval --
+(_|_){
+  // [incomplete] incomplete bool: _
+  _request: (struct){
+    enabled: (_){ _ }
+  }
+}
+-- out/compile --
+--- in.cue
+{
+  if 〈0;_request〉.enabled {
+    r1: {
+      k1: 0
+      k2: 0
+      k3: 0
+      k4: 0
+      k5: 0
+      k6: 0
+      k7: 0
+      k8: 0
+      k9: 0
+      k10: 0
+      k11: 0
+      k12: 0
+      k13: 0
+      k14: 0
+      k15: 0
+      k16: 0
+      k17: 0
+    }
+  }
+  _request: {
+    enabled: _
+  }
+}

cue/testdata/eval/dynamic_field.txtar

@@ -164,7 +164,7 @@ Result:
           // [incomplete] noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string):
           //     ./in.cue:59:8
           //     ./in.cue:59:11
-          // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string) (and 1 more errors)):
+          // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string)):
           //     ./in.cue:61:22
         }
         #ID: (_|_){
@@ -201,7 +201,7 @@ diff old new
            // [incomplete] noCycleError.foo.baz.#ID: invalid interpolation: non-concrete value string (type string):
            //     ./in.cue:59:8
            //     ./in.cue:59:11
-+          // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string) (and 1 more errors)):
++          // noCycleError.foo.bar.entries: key value of dynamic field must be concrete, found _|_(invalid interpolation: noCycleError.foo.baz.#ID: non-concrete value string (type string)):
 +          //     ./in.cue:61:22
          }
          #ID: (_|_){

internal/core/export/testdata/main/adt.txtar

@@ -337,119 +337,6 @@ Reordering.
 [errorListDef #Def]
 [errorListDef 0]
 [errorListDef 1]
--- out/value-v3 --
-== Simplified
-_|_ // e3: index out of range [2] with length 2
-== Raw
-_|_ // e3: index out of range [2] with length 2
-== Final
-_|_ // e3: index out of range [2] with length 2
-== All
-{
-	@foo(bar)
-	p1: {}
-	d1: {
-		foobar: int
-	}
-	bar: "bar"
-	d2: {
-		foobar: {
-			name: "xx"
-			foo:  "xx"
-		}
-	}
-
-	// Issue #1910
-	a: _
-	comp: {
-		for k, v in [0]
-		let w = v {
-			"\(a)": w
-			"bar":  w
-		}
-	}
-	bytes: '\xeb \x1a\xf5\xaa\xf0\xd6\x06)'
-	c1:    true
-	s1: """
-		multi
-		bar
-		line
-		"""
-	l1: [3, ...int]
-	l2: [...int]
-	l3: []
-	l4: [1, 2]
-	l5: {
-		#foo: int
-		[1, 3]
-	}
-	#foo: int
-	l6: {
-		#foo: int
-		[1, 3]
-	}
-	n1:  1.0
-	n10: 10
-
-	// t is true
-	t:   true
-	e1:  <1.0
-	e2:  >1.0 & <10
-	e3:  _|_ // e3: index out of range [2] with length 2
-	e4:  _|_ // e4: index 3 out of range (and 1 more errors)
-	e5:  _|_ // e3: index out of range [2] with length 2 (and 1 more errors)
-	e6:  false
-	e7:  true
-	e8?: true
-	m1: {
-		// foo is an optional field
-		foo?: 3
-
-		// bar is a field
-		bar: 4
-
-		// baz is a required field.
-		baz!: 5
-	}
-	y1: {
-		src: [1, 2, 3]
-		foo0: 1
-		bar1: 2
-		bar2: 3
-		foo1: 2
-		x: [1, 2, 3]
-		foo2: 3
-	}
-	preserveKeyFieldInComprehension: 1
-	errorStructDef: {
-		a:    1
-		b:    _|_ // errorStructDef.b: conflicting values 2 and 1
-		#Def: 1
-	}
-	errorList: [1, _|_]
-	x: int
-	errorListDef: {
-		#Def: 1
-		[1, _|_]
-	}
-}
-== Eval
-_|_ // e3: index out of range [2] with length 2
--- diff/-out/value-v3<==>+out/value --
-diff old new
---- old
-+++ new
-@@ -56,8 +56,8 @@
- 	e1:  <1.0
- 	e2:  >1.0 & <10
- 	e3:  _|_ // e3: index out of range [2] with length 2
--	e4:  _|_ // e4: index 3 out of range
--	e5:  _|_ // e3: index out of range [2] with length 2
-+	e4:  _|_ // e4: index 3 out of range (and 1 more errors)
-+	e5:  _|_ // e3: index out of range [2] with length 2 (and 1 more errors)
- 	e6:  false
- 	e7:  true
- 	e8?: true
 -- diff/value/todo/p3 --
 Error message change.
 -- out/value --

internal/core/export/testdata/main/let.txtar

@@ -583,7 +583,7 @@ y: Y & Y_1
 }
 == Final
 {
-	comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: comprehension.filepath: undefined field: name (and 1 more errors)) (and 2 more errors)
+	comprehension: _|_ // comprehension: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: comprehension.filepath: undefined field: name) (and 1 more errors)
 	complete: {
 		x: "a foo z"
 		run: {
@@ -599,7 +599,7 @@ y: Y & Y_1
 			name: "two"
 		}
 	}]
-	files: _|_ // files: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: filepath: undefined field: name (and 1 more errors)) (and 5 more errors)
+	files: _|_ // files: key value of dynamic field must be concrete, found _|_(invalid interpolation: invalid interpolation: filepath: undefined field: name) (and 3 more errors)
 	scoped: {
 		direct: {
 			a: 1
@@ -625,20 +625,20 @@ y: Y & Y_1
 	x: "foo"
 	incomplete: {
 		a: {
-			x: _|_ // invalid interpolation: incomplete.a.x: non-concrete value string (type string) (and 1 more errors)
+			x: _|_ // invalid interpolation: incomplete.a.x: non-concrete value string (type string)
 			run: {
 				a: string
 			}
 		}
 		b: {
-			x: _|_ // invalid interpolation: incomplete.b.x: non-concrete value string (type string) (and 1 more errors)
+			x: _|_ // invalid interpolation: incomplete.b.x: non-concrete value string (type string)
 			run: {
 				a: string
 			}
 		}
 		c: {
-			x:  _|_ // invalid interpolation: incomplete.c.x: non-concrete value string (type string) (and 1 more errors)
-			x2: _|_ // invalid interpolation: incomplete.c.x2: non-concrete value string (type string) (and 1 more errors)
+			x:  _|_ // invalid interpolation: incomplete.c.x: non-concrete value string (type string)
+			x2: _|_ // invalid interpolation: incomplete.c.x2: non-concrete value string (type string)
 			run: {
 				a: string
 			}
@@ -647,8 +647,8 @@ y: Y & Y_1
 			}
 		}
 		d: {
-			x:  _|_ // invalid interpolation: incomplete.d.x: non-concrete value string (type string) (and 1 more errors)
-			x2: _|_ // invalid interpolation: incomplete.d.x2: non-concrete value string (type string) (and 1 more errors)
+			x:  _|_ // invalid interpolation: incomplete.d.x: non-concrete value string (type string)
+			x2: _|_ // invalid interpolation: incomplete.d.x2: non-concrete value string (type string)
 			run: {
 				a: string
 			}
@@ -895,7 +895,7 @@ y: Y & Y_1
 diff old new
 --- old
 +++ new
-@@ -216,72 +216,72 @@
+@@ -216,23 +216,23 @@
  }
  == Final
  {

internal/core/export/testdata/main/simplify.txtar

@@ -39,83 +39,6 @@ additional: {
 [additional]
 [additional env]
 [additional confs]
--- out/value-v3 --
-== Simplified
-{
-	x: {
-		y: int64
-	}
-	s: strings.MinRunes(4) & strings.MaxRunes(7)
-	additional: {
-		env: _
-		confs: {
-			if env {}
-		}
-	}
-}
-== Raw
-{
-	x: {
-		y: >=-9223372036854775808 & <=9223372036854775807 & int
-	}
-	s: strings.MinRunes(4) & strings.MaxRunes(7)
-	additional: {
-		env: _
-		confs: {
-			if env {}
-		}
-	}
-}
-== Final
-{
-	x: {
-		y: int64
-	}
-	s: strings.MinRunes(4) & strings.MaxRunes(7)
-	additional: {
-		env:   _
-		confs: _|_ // additional.confs: incomplete bool: _ (and 1 more errors)
-	}
-}
-== All
-{
-	x: {
-		y: int64
-	}
-	s: strings.MinRunes(4) & strings.MaxRunes(7)
-	additional: {
-		env: _
-		confs: {
-			if env {}
-		}
-	}
-}
-== Eval
-{
-	x: {
-		y: >=-9223372036854775808 & <=9223372036854775807 & int
-	}
-	s: strings.MinRunes(4) & strings.MaxRunes(7)
-	additional: {
-		env: _
-		confs: {
-			if env {}
-		}
-	}
-}
--- diff/-out/value-v3<==>+out/value --
-diff old new
---- old
-+++ new
-@@ -32,7 +32,7 @@
- 	s: strings.MinRunes(4) & strings.MaxRunes(7)
- 	additional: {
- 		env:   _
--		confs: _|_ // additional.confs: incomplete bool: _ (and 2 more errors)
-+		confs: _|_ // additional.confs: incomplete bool: _ (and 1 more errors)
- 	}
- }
- == All
 -- diff/value/explanation --
 Benign change in error message.
 -- out/value --
@@ -153,7 +76,7 @@ Benign change in error message.
 	s: strings.MinRunes(4) & strings.MaxRunes(7)
 	additional: {
 		env:   _
-		confs: _|_ // additional.confs: incomplete bool: _ (and 2 more errors)
+		confs: _|_ // additional.confs: incomplete bool: _
 	}
 }
 == All