Skip to content

Commit

Permalink
internal/core/adt: sharpen condition for scalar setting
Browse files Browse the repository at this point in the history
Fixes some issues caused by a regression in v0.5.
It is true that a scalar should not discard an error,
but it may override other values, like basic types
or other values of which a concrete scalar may be
an instance of.

Issue #2244

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I1351b250683f4f17e78cf5974f9423ee081fe568
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/549657
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
mpvl committed Mar 14, 2023
1 parent b44e16d commit 9b1f222
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
8 changes: 4 additions & 4 deletions cue/testdata/eval/insertion.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ Disjuncts: 77
embeddingDirect: (struct){
t1: (string){
"s"
#a: (string){ string }
#a: (string){ "s" }
}
}
embeddingExpr: (struct){
t1: (string){
"s"
#a: (string){ string }
#a: (string){ "s" }
}
}
unifiedDirect: (struct){
Expand Down Expand Up @@ -188,9 +188,9 @@ Disjuncts: 77
"str"
#fn: (string){
"str"
#in: (string){ string }
#in: (string){ "str" }
}
#in: (string){ string }
#in: (string){ "str" }
}
}
-- out/compile --
Expand Down
3 changes: 2 additions & 1 deletion internal/core/adt/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,8 @@ func (n *nodeContext) getValidators(state VertexStatus) BaseValue {
func (n *nodeContext) maybeSetCache() {
// Set BaseValue to scalar, but only if it was not set before. Most notably,
// errors should not be discarded.
if n.scalar != nil && isCyclePlaceholder(n.node.BaseValue) {
_, isErr := n.node.BaseValue.(*Bottom)
if n.scalar != nil && (!isErr || isCyclePlaceholder(n.node.BaseValue)) {
n.node.BaseValue = n.scalar
}
// NOTE: this is now handled by associating the nodeContext
Expand Down

0 comments on commit 9b1f222

Please sign in to comment.