Skip to content

Commit 053f47b

Browse files
mpvlmpvl
committed
internal/core/adt: fix constraints deduplication
Basically, findConjuncts did not check for equality of closeContext. For other uses of findConjuncts, this tended to be okay, as the Env and Vertex would disambiguate it. But these are not used for constraints. In case of the use of constraints.go:104, this led to pattern constraints being dropped that really needed to be added to ensure closedness information is maintained. For constraints specifically, we now use VisitLeafConjuncts instead. We could just use a simple loop, but this is a bit more future proof. Test changes: closed.txtar: diff mostly due to reordering: is now very close to V2 bulk.txtar: the actual fix. Note that V3 is now seemingly even more different from V2. The answer of V3 is correct and V2 has multiple bugs. See explanation. Also note that issue3638.compare and .reduced tests now give the same results, as one might expect. Fixes #3638 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: Ia3cb72694951c0608fdaa935a49a7a1215194d20 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207410 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent b1eaedd commit 053f47b

File tree

5 files changed

+149
-119
lines changed

5 files changed

+149
-119
lines changed

Diff for: ‎cue/testdata/builtins/closed.txtar

+49-36
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,38 @@ issue3580: {
5656
b: x.a
5757
})
5858
}
59+
-- out/evalalpha/stats --
60+
Leaks: 372
61+
Freed: 6
62+
Reused: 6
63+
Allocs: 372
64+
Retain: 0
65+
66+
Unifications: 179
67+
Conjuncts: 913
68+
Disjuncts: 24
69+
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
70+
diff old new
71+
--- old
72+
+++ new
73+
@@ -1,9 +1,9 @@
74+
-Leaks: 43
75+
-Freed: 170
76+
-Reused: 164
77+
-Allocs: 49
78+
-Retain: 44
79+
+Leaks: 372
80+
+Freed: 6
81+
+Reused: 6
82+
+Allocs: 372
83+
+Retain: 0
84+
85+
-Unifications: 199
86+
-Conjuncts: 389
87+
-Disjuncts: 204
88+
+Unifications: 179
89+
+Conjuncts: 913
90+
+Disjuncts: 24
5991
-- out/eval/stats --
6092
Leaks: 43
6193
Freed: 170
@@ -127,7 +159,7 @@ Result:
127159
a: (#struct){
128160
b: (bool){ true }
129161
}
130-
let X#1 = ~(inDisjunctions.x.syslog.xxx.a)
162+
let X#1multi = 〈0;a〉
131163
uint: (#struct){
132164
a: (#struct){
133165
b: (bool){ true }
@@ -137,7 +169,7 @@ Result:
137169
a: (#struct){
138170
b: (bool){ true }
139171
}
140-
let X#1 = ~(inDisjunctions.x.syslog.xxx.a)
172+
let X#1multi = 〈0;a〉
141173
string: (#struct){
142174
a: (#struct){
143175
b: (bool){ true }
@@ -148,7 +180,7 @@ Result:
148180
a: (#struct){
149181
b: (bool){ true }
150182
}
151-
let X#1 = ~(inDisjunctions.x.syslog.string.a)
183+
let X#1multi = 〈0;a〉
152184
uint: (#struct){
153185
a: (#struct){
154186
b: (bool){ true }
@@ -158,7 +190,7 @@ Result:
158190
a: (#struct){
159191
b: (bool){ true }
160192
}
161-
let X#1 = ~(inDisjunctions.x.syslog.string.a)
193+
let X#1multi = 〈0;a〉
162194
string: (#struct){
163195
a: (#struct){
164196
b: (bool){ true }
@@ -283,7 +315,7 @@ diff old new
283315
string: (#struct){
284316
a: (#struct){
285317
b: (bool){ true }
286-
@@ -60,42 +54,42 @@
318+
@@ -60,27 +54,6 @@
287319
}) }
288320
}
289321
syslog: (#struct){
@@ -311,19 +343,15 @@ diff old new
311343
xxx: (#struct){ |((#struct){
312344
a: (#struct){
313345
b: (bool){ true }
346+
@@ -102,6 +75,27 @@
347+
}
314348
}
315-
- let X#1multi = 〈0;a〉
316-
- uint: (#struct){
317-
- a: (#struct){
318-
- b: (bool){ true }
319-
- }
320-
- }
321-
- }, (#struct){
322-
- a: (#struct){
323-
- b: (bool){ true }
324-
- }
325-
- let X#1multi = 〈0;a〉
326-
+ let X#1 = ~(inDisjunctions.x.syslog.xxx.a)
349+
}) }
350+
+ string: (#struct){ |((#struct){
351+
+ a: (#struct){
352+
+ b: (bool){ true }
353+
+ }
354+
+ let X#1multi = 〈0;a〉
327355
+ uint: (#struct){
328356
+ a: (#struct){
329357
+ b: (bool){ true }
@@ -333,31 +361,16 @@ diff old new
333361
+ a: (#struct){
334362
+ b: (bool){ true }
335363
+ }
336-
+ let X#1 = ~(inDisjunctions.x.syslog.xxx.a)
364+
+ let X#1multi = 〈0;a〉
337365
+ string: (#struct){
338366
+ a: (#struct){
339367
+ b: (bool){ true }
340368
+ }
341369
+ }
342370
+ }) }
343-
+ string: (#struct){ |((#struct){
344-
+ a: (#struct){
345-
+ b: (bool){ true }
346-
+ }
347-
+ let X#1 = ~(inDisjunctions.x.syslog.string.a)
348-
+ uint: (#struct){
349-
+ a: (#struct){
350-
+ b: (bool){ true }
351-
+ }
352-
+ }
353-
+ }, (#struct){
354-
+ a: (#struct){
355-
+ b: (bool){ true }
356-
+ }
357-
+ let X#1 = ~(inDisjunctions.x.syslog.string.a)
358-
string: (#struct){
359-
a: (#struct){
360-
b: (bool){ true }
371+
}
372+
}
373+
#Def: (#struct){
361374
-- diff/todo/p3 --
362375
Reordering
363376
Let differs.

Diff for: ‎cue/testdata/cycle/comprehension.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ Allocs: 782
317317
Retain: 0
318318

319319
Unifications: 502
320-
Conjuncts: 3031
320+
Conjuncts: 3030
321321
Disjuncts: 196
322322
-- out/evalalpha --
323323
Errors:
@@ -881,7 +881,7 @@ diff old new
881881
-Conjuncts: 2525
882882
-Disjuncts: 1404
883883
+Unifications: 502
884-
+Conjuncts: 3031
884+
+Conjuncts: 3030
885885
+Disjuncts: 196
886886
-- out/eval/stats --
887887
Leaks: 50

Diff for: ‎cue/testdata/eval/bulk.txtar

+82-78
Original file line numberDiff line numberDiff line change
@@ -355,21 +355,13 @@ Result:
355355
0: (#struct){
356356
route: (#struct){
357357
default: (#struct){
358-
spec: (#list){ |((#list){
359-
0: (#struct){
360-
refs: (#list){
361-
0: (string){ "default" }
362-
}
363-
}
364-
}, (#list){
365-
0: (#struct){
366-
refs: (#list){
367-
0: (string){ "default" }
368-
}
369-
other: (#list){
370-
}
358+
spec: (#list){
359+
0: (#struct){
360+
refs: (#list){
361+
0: (string){ "default" }
371362
}
372-
}) }
363+
}
364+
}
373365
}
374366
}
375367
}
@@ -385,12 +377,9 @@ Result:
385377
}
386378
}
387379
out: (struct){
388-
foo: (#struct){ |((#struct){
389-
a: (int){ 1 }
390-
}, (#struct){
391-
a: (int){ 1 }
392-
b: (int){ 2 }
393-
}) }
380+
foo: (#struct){
381+
a: (int){ 1 }
382+
}
394383
}
395384
#x: (#struct){ |(*(#struct){
396385
b: (int){ 2 }
@@ -678,7 +667,7 @@ diff old new
678667
}
679668
}
680669
}
681-
@@ -232,22 +169,21 @@
670+
@@ -232,60 +169,8 @@
682671
}, (list){
683672
}) }
684673
}
@@ -699,69 +688,85 @@ diff old new
699688
- }
700689
+ entrypoint: ~(issue3638.full.used.output)
701690
used: (#struct){
702-
+ Routes: (#struct){
703-
+ route: (#struct){
704-
+ default: (#struct){
705-
+ spec: (#list){
706-
+ 0: (#struct){
707-
+ refs: (#list){
708-
+ 0: (string){ "default" }
709-
+ }
710-
+ }
711-
+ }
712-
+ }
713-
+ }
714-
+ }
715-
input: (#struct){
716-
res1: (#struct){
717-
route: (#struct){
718-
@@ -267,12 +203,10 @@
719-
0: (#struct){
720-
route: (#struct){
721-
default: (#struct){
691+
- input: (#struct){
692+
- res1: (#struct){
693+
- route: (#struct){
694+
- default: (#struct){
695+
- spec: (#list){
696+
- 0: (#struct){
697+
- refs: (#list){
698+
- 0: (string){ "default" }
699+
- }
700+
- }
701+
- }
702+
- }
703+
- }
704+
- }
705+
- }
706+
- output: (#list){
707+
- 0: (#struct){
708+
- route: (#struct){
709+
- default: (#struct){
722710
- spec: (list){ |(*(#list){
723711
- 0: (#struct){
724712
- refs: (#list){
725713
- 0: (string){ "default" }
726714
- }
727715
- other: (#list){
728-
+ spec: (#list){ |((#list){
729-
+ 0: (#struct){
730-
+ refs: (#list){
731-
+ 0: (string){ "default" }
732-
}
733-
}
734-
}, (#list){
735-
@@ -280,6 +214,8 @@
736-
refs: (#list){
737-
0: (string){ "default" }
738-
}
739-
+ other: (#list){
740-
+ }
741-
}
742-
}) }
743-
}
744-
@@ -286,19 +222,6 @@
745-
}
746-
}
747-
}
748-
- Routes: (#struct){
749-
- route: (#struct){
750-
- default: (#struct){
751-
- spec: (#list){
752-
- 0: (#struct){
753-
- refs: (#list){
754-
- 0: (string){ "default" }
755-
- }
756-
- }
716+
- }
717+
- }
718+
- }, (#list){
719+
- 0: (#struct){
720+
- refs: (#list){
721+
- 0: (string){ "default" }
722+
- }
723+
- }
724+
- }) }
757725
- }
758726
- }
759727
- }
760728
- }
729+
Routes: (#struct){
730+
route: (#struct){
731+
default: (#struct){
732+
@@ -299,6 +184,36 @@
733+
}
734+
}
735+
}
736+
+ input: (#struct){
737+
+ res1: (#struct){
738+
+ route: (#struct){
739+
+ default: (#struct){
740+
+ spec: (#list){
741+
+ 0: (#struct){
742+
+ refs: (#list){
743+
+ 0: (string){ "default" }
744+
+ }
745+
+ }
746+
+ }
747+
+ }
748+
+ }
749+
+ }
750+
+ }
751+
+ output: (#list){
752+
+ 0: (#struct){
753+
+ route: (#struct){
754+
+ default: (#struct){
755+
+ spec: (#list){
756+
+ 0: (#struct){
757+
+ refs: (#list){
758+
+ 0: (string){ "default" }
759+
+ }
760+
+ }
761+
+ }
762+
+ }
763+
+ }
764+
+ }
765+
+ }
761766
}
762767
}
763768
reduced: (struct){
764-
@@ -310,11 +233,11 @@
769+
@@ -310,12 +225,9 @@
765770
}
766771
}
767772
out: (struct){
@@ -770,15 +775,14 @@ diff old new
770775
- b: (int){ 2 }
771776
- }, (#struct){
772777
- a: (int){ 1 }
773-
+ foo: (#struct){ |((#struct){
774-
+ a: (int){ 1 }
775-
+ }, (#struct){
776-
+ a: (int){ 1 }
777-
+ b: (int){ 2 }
778-
}) }
778+
- }) }
779+
+ foo: (#struct){
780+
+ a: (int){ 1 }
781+
+ }
779782
}
780783
#x: (#struct){ |(*(#struct){
781-
@@ -332,12 +255,9 @@
784+
b: (int){ 2 }
785+
@@ -332,12 +244,9 @@
782786
}
783787
}
784788
out: (struct){

Diff for: ‎internal/core/adt/composite.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -1366,10 +1366,13 @@ func (v *Vertex) hasConjunct(c Conjunct) (added bool) {
13661366
}
13671367

13681368
// findConjunct reports the position of c within cs or -1 if it is not found.
1369+
//
1370+
// NOTE: we are not comparing closeContexts. The intended use of this function
1371+
// is only to add to list of conjuncts within a closeContext.
13691372
func findConjunct(cs []Conjunct, c Conjunct) int {
13701373
for i, x := range cs {
13711374
// TODO: disregard certain fields from comparison (e.g. Refs)?
1372-
if x.CloseInfo.closeInfo == c.CloseInfo.closeInfo &&
1375+
if x.CloseInfo.closeInfo == c.CloseInfo.closeInfo && // V2
13731376
x.x == c.x &&
13741377
x.Env.Up == c.Env.Up && x.Env.Vertex == c.Env.Vertex {
13751378
return i

0 commit comments

Comments
 (0)