Skip to content

Commit

Permalink
internal/core/adt: match patterns only if type matches
Browse files Browse the repository at this point in the history
Failing to add this check may lead to incorrect errors.
An alterantive fix would be to check for each case in
the switch statement whether the type matches. Some of
them do, but not all. Easier seems to do a catch all
at the top.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Iab4f0e043439a654747e9f545cda8fa561686135
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1191573
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mpvl committed Apr 12, 2024
1 parent 50989f0 commit 8e58091
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 45 deletions.
62 changes: 20 additions & 42 deletions cue/testdata/comprehensions/iferror.txtar
Expand Up @@ -158,24 +158,20 @@ Conjuncts: 66
Disjuncts: 57
-- out/evalalpha --
Errors:
issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
./in.cue:61:8
./in.cue:63:11
./in.cue:63:15
issue1972.err1.patchs: field not allowed:
./in.cue:63:3
./in.cue:63:15
./in.cue:66:4
issue1972.err1.someCondition: field not allowed:
./in.cue:62:3
./in.cue:63:15
wrongConcreteType: cannot use 2 (type int) as type bool:
./in.cue:4:2
./in.cue:1:8
wrongType: cannot use int (type int) as type bool:
./in.cue:10:2
./in.cue:1:14
issue1972.err1: cannot use {} (type struct) as type bool:
./in.cue:65:3

Result:
(_|_){
Expand Down Expand Up @@ -230,29 +226,23 @@ Result:
issue1972: (_|_){
// [eval]
err1: (_|_){
// [eval] issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
// [eval] issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
// ./in.cue:61:8
// ./in.cue:63:11
// ./in.cue:63:15
// issue1972.err1: cannot use {} (type struct) as type bool:
// ./in.cue:65:3
#patchs: (#list){
}
#someCondition: (_|_){
// [eval] issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
// [eval] issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
// ./in.cue:61:8
// ./in.cue:63:11
// ./in.cue:63:15
// issue1972.err1: cannot use {} (type struct) as type bool:
// ./in.cue:65:3
}
someCondition: (_|_){
// [eval] issue1972.err1.someCondition: field not allowed:
// ./in.cue:62:3
// ./in.cue:63:15
}
patchs: (_|_){
// [eval] issue1972.err1.patchs: field not allowed:
// ./in.cue:63:3
// ./in.cue:63:15
// ./in.cue:66:4
}
}
Expand All @@ -262,36 +252,31 @@ Result:
diff old new
--- old
+++ new
@@ -1,7 +1,14 @@
@@ -1,7 +1,12 @@
Errors:
-issue1972.err1: conflicting values [] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
- ./in.cue:54:12
- ./in.cue:61:8
+issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
+issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
./in.cue:61:8
+ ./in.cue:63:11
+ ./in.cue:63:15
+issue1972.err1.patchs: field not allowed:
+ ./in.cue:63:3
+ ./in.cue:63:15
+ ./in.cue:66:4
+issue1972.err1.someCondition: field not allowed:
+ ./in.cue:62:3
+ ./in.cue:63:15
wrongConcreteType: cannot use 2 (type int) as type bool:
./in.cue:4:2
./in.cue:1:8
@@ -8,8 +15,8 @@
@@ -8,8 +13,6 @@
wrongType: cannot use int (type int) as type bool:
./in.cue:10:2
./in.cue:1:14
-issue1972.err1: invalid list index someCondition (type string):
- ./in.cue:65:6
+issue1972.err1: cannot use {} (type struct) as type bool:
+ ./in.cue:65:3

Result:
(_|_){
@@ -31,8 +38,6 @@
@@ -31,8 +34,6 @@
incomplete: (_|_){
// [incomplete] incomplete: undefined field: d:
// ./in.cue:16:7
Expand All @@ -300,51 +285,44 @@ diff old new
list: (#list){
0: (int){ 1 }
1: (int){ 2 }
@@ -66,17 +71,31 @@
@@ -66,17 +67,25 @@
issue1972: (_|_){
// [eval]
err1: (_|_){
- // [eval] issue1972.err1: conflicting values [] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
- // ./in.cue:54:12
- // ./in.cue:61:8
+ // [eval] issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
// ./in.cue:61:8
- // issue1972.err1: invalid list index someCondition (type string):
- // ./in.cue:65:6
+ // [eval] issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
+ // ./in.cue:63:11
+ // ./in.cue:63:15
+ // issue1972.err1: cannot use {} (type struct) as type bool:
+ // ./in.cue:65:3
#patchs: (#list){
}
- someCondition: (_){ _ }
- patchs: (list){
- }
- #someCondition: (_){ _ }
+ #someCondition: (_|_){
+ // [eval] issue1972.err1: conflicting values [...{}] and {} (mismatched types list and struct):
+ // [eval] issue1972.err1: conflicting values [...{}] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
+ // ./in.cue:61:8
+ // ./in.cue:63:11
+ // ./in.cue:63:15
+ // issue1972.err1: cannot use {} (type struct) as type bool:
+ // ./in.cue:65:3
+ }
+ someCondition: (_|_){
+ // [eval] issue1972.err1.someCondition: field not allowed:
+ // ./in.cue:62:3
+ // ./in.cue:63:15
+ }
+ patchs: (_|_){
+ // [eval] issue1972.err1.patchs: field not allowed:
+ // ./in.cue:63:3
+ // ./in.cue:63:15
+ // ./in.cue:66:4
+ }
}
}
}
-- diff/todo/p0 --
issue1972: incorrect errors: "field not allowed" when they are actually allowed.
-- diff/todo/p1 --
incomplete: missing error (second incomplete block)
-- diff/todo/p2 --
issue1972: "field not allowed" are somewhat correct (it is also a list), but
are already covered by the other error message and should be elided.
This may lead to a large number of errors otherwise.
-- out/eval --
Errors:
issue1972.err1: conflicting values [] and {someCondition:_,patchs:[...{}],patchs,if someCondition {patchs:_}} (mismatched types list and struct):
Expand Down
8 changes: 8 additions & 0 deletions internal/core/adt/constraints.go
Expand Up @@ -143,6 +143,14 @@ func matchPatternValue(ctx *OpContext, pattern Value, f Feature, label Value) (r
return true
}

k := IntKind
if label != nil {
k = label.Kind()
}
if !k.IsAnyOf(pattern.Kind()) {
return false
}

// Fast track for the majority of cases.
switch x := pattern.(type) {
case *Bottom:
Expand Down
8 changes: 5 additions & 3 deletions internal/core/adt/eval_test.go
Expand Up @@ -74,12 +74,14 @@ func TestEvalAlpha(t *testing.T) {

var todoAlpha = map[string]string{
// Crashes and hangs
"cycle/chain": "hang",
"cycle/evaluate": "hang",
"disjunctions/elimination": "panic (nil pointer)",
"cycle/chain": "hang",
"cycle/evaluate": "hang",

// Later
"benchmarks/issue2176": "fails to remove errors",

// Performance
"disjunctions/elimination": "performance issue",
}

test := cuetxtar.TxTarTest{
Expand Down

0 comments on commit 8e58091

Please sign in to comment.