internal/core: fix incorrect error handling
1: don't double-wrap errors in BinOp. This could happen
when a value is passed as a Vertex in API usage.
2: subsumption incorrectly used And op for BinOp.
Together these Fixes #461
This also improves reporting of structural errors,
although this is not complete and not the goal of
this CL.
Change-Id: Id021a0eb4fa5628f56167d2fccea7d99dbddf390
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7065
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/script/issue461.txt b/cmd/cue/cmd/testdata/script/issue461.txt
new file mode 100644
index 0000000..a95d002
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/issue461.txt
@@ -0,0 +1,26 @@
+cue cmd writefile
+
+-- cue.mod/module.cue --
+module: "mod.com"
+-- expect-hello --
+Hello, world!
+-- x_tool.cue --
+package x
+
+import (
+ "strings"
+ "tool/exec"
+ "tool/file"
+)
+
+command: writefile: {
+ echo: exec.Run & {
+ cmd: "echo hello.txt"
+ stdout: string
+ }
+
+ write: file.Create & {
+ filename: strings.TrimSpace(echo.stdout)
+ contents: "Hello, world!"
+ }
+}
diff --git a/cue/testdata/cycle/structural.txtar b/cue/testdata/cycle/structural.txtar
index 678384c..0f76200 100644
--- a/cue/testdata/cycle/structural.txtar
+++ b/cue/testdata/cycle/structural.txtar
@@ -84,7 +84,7 @@
}
d3: {
- // TODO(errors): position reporting in structural cycle.
+ // TODO(errors): correct position reporting in structural cycle.
config: {
a: b: c: indirect
indirect: [a.b, null][i]
@@ -253,6 +253,8 @@
./in.cue:79:8
d2.r: structural cycle:
./in.cue:79:8
+0: structural cycle:
+ ./in.cue:89:19
Result:
(_|_){
@@ -507,12 +509,14 @@
b: (_|_){
// [structural cycle]
c: (_|_){
- // [structural cycle]
+ // [structural cycle] 0: structural cycle:
+ // ./in.cue:89:19
}
}
}
indirect: (_|_){
- // [structural cycle]
+ // [structural cycle] 0: structural cycle:
+ // ./in.cue:89:19
}
i: (int){ 1 }
}
@@ -523,12 +527,14 @@
b: (_|_){
// [structural cycle]
c: (_|_){
- // [structural cycle]
+ // [structural cycle] 0: structural cycle:
+ // ./in.cue:89:19
}
}
}
indirect: (_|_){
- // [structural cycle]
+ // [structural cycle] 0: structural cycle:
+ // ./in.cue:89:19
}
i: (int){ 0 }
}
diff --git a/internal/core/adt/binop.go b/internal/core/adt/binop.go
index 7edd191..b43bea6 100644
--- a/internal/core/adt/binop.go
+++ b/internal/core/adt/binop.go
@@ -51,11 +51,8 @@
}
}
- if a, ok := left.(*Bottom); ok {
- return CombineErrors(nil, a, right)
- }
- if b, ok := left.(*Bottom); ok {
- return b
+ if err := CombineErrors(c.src, left, right); err != nil {
+ return err
}
switch op {
diff --git a/internal/core/adt/errors.go b/internal/core/adt/errors.go
index 4736dbb..1f54c86 100644
--- a/internal/core/adt/errors.go
+++ b/internal/core/adt/errors.go
@@ -171,8 +171,8 @@
// CombineErrors combines two errors that originate at the same Vertex.
func CombineErrors(src ast.Node, x, y Value) *Bottom {
- a, _ := x.(*Bottom)
- b, _ := y.(*Bottom)
+ a, _ := Unwrap(x).(*Bottom)
+ b, _ := Unwrap(y).(*Bottom)
switch {
case a != nil && b != nil:
diff --git a/internal/core/subsume/subsume.go b/internal/core/subsume/subsume.go
index 9ce210a..ba00aab 100644
--- a/internal/core/subsume/subsume.go
+++ b/internal/core/subsume/subsume.go
@@ -103,6 +103,13 @@
s.errs = errors.Append(s.errs, b.Err)
}
+func unifyValue(c *adt.OpContext, a, b adt.Value) adt.Value {
+ v := &adt.Vertex{}
+ v.AddConjunct(adt.MakeRootConjunct(c.Env(0), a))
+ v.AddConjunct(adt.MakeRootConjunct(c.Env(0), b))
+ return c.Unifier.Evaluate(c, v)
+}
+
func (s *subsumer) getError() (err errors.Error) {
c := s.ctx
// src := binSrc(token.NoPos, opUnify, gt, lt)
@@ -110,7 +117,7 @@
// src := binSrc(token.NoPos, opUnify, s.gt, s.lt)
if s.missing != 0 {
s.errf("missing field %q", s.missing.SelectorString(c))
- } else if b, ok := adt.BinOp(c, adt.AndOp, s.gt, s.lt).(*adt.Bottom); !ok {
+ } else if b, ok := unifyValue(c, s.gt, s.lt).(*adt.Bottom); !ok {
s.errf("value not an instance")
} else {
s.errs = errors.Append(s.errs, b.Err)
