this repo has no description
internal/core/adt: prevent early decrements in comprehensions
Pending arcs in comprehensions follow a slightly
different code path and are evaluated a bit earlier
to determine whether the fields will exist.
We need to ensure that if a "complete all" path is
agitated, we do not break incoming dependencies
if we are not finalizing. Doing so might cause the
closeContexts to be closed, causing an "already
closed" panic when a matching pattern constraint
is later evaluated.
We enforce this by requiring `breakIncomingDeps`
to take the current mode and then only commence
when mode is finalize.
Note that there are two call sites. We added two
test cases, the reported one and a variant, each of
which triggered a panic for the respective code paths.
Fixes #3691
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Ib4e9dda97a002f2ac5f96f4b31985e6c7ddb4f07
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207545
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
+349
-11
+338
-8
cue/testdata/eval/comprehensions.txtar
+338
-8
cue/testdata/eval/comprehensions.txtar
···
96
96
out: [string]: Val: 1
97
97
}
98
98
}
99
-
99
+
-- issue3691.cue --
100
+
// Ensure that parent nodes are properly processed, even when a pending arc
101
+
// is evaluated early.
102
+
issue3691: original: {
103
+
X: [string]: string
104
+
a: [string]: X
105
+
a: {
106
+
if true {
107
+
b: c: 1
108
+
}
109
+
}
110
+
}
111
+
// Structural cycles follow a different code path.
112
+
issue3691: structuralCycle: {
113
+
X: [string]: string
114
+
a: [string]: X
115
+
a: {
116
+
if true {
117
+
b: c: a
118
+
}
119
+
}
120
+
}
100
121
-- out/eval/stats --
101
122
Leaks: 4
102
-
Freed: 64
103
-
Reused: 57
104
-
Allocs: 11
123
+
Freed: 77
124
+
Reused: 69
125
+
Allocs: 12
105
126
Retain: 19
106
127
107
-
Unifications: 68
108
-
Conjuncts: 96
109
-
Disjuncts: 81
128
+
Unifications: 81
129
+
Conjuncts: 121
130
+
Disjuncts: 94
131
+
-- out/evalalpha --
132
+
Errors:
133
+
issue3691.original.a.b: conflicting values 1 and string (mismatched types int and string):
134
+
./issue3691.cue:4:15
135
+
./issue3691.cue:8:10
136
+
issue3691.structuralCycle.a.b.c: structural cycle
137
+
138
+
Result:
139
+
(_|_){
140
+
// [eval]
141
+
a: (struct){
142
+
x: (int){ 10 }
143
+
y: (int){ 100 }
144
+
z: (int){ 50 }
145
+
}
146
+
b: (struct){
147
+
x: (int){ 10 }
148
+
k: (int){ 20 }
149
+
l: (int){ 40 }
150
+
z: (int){ 50 }
151
+
}
152
+
c: (struct){
153
+
y: (int){ 110 }
154
+
z: (int){ 60 }
155
+
}
156
+
A: (struct){
157
+
X: (struct){
158
+
run: (string){ "dfoo" }
159
+
files: (string){ "dfoo" }
160
+
}
161
+
}
162
+
matchOrder: (struct){
163
+
a1: (struct){
164
+
out: (struct){
165
+
b: (struct){
166
+
Val: (int){ 1 }
167
+
}
168
+
}
169
+
in: (struct){
170
+
a: (struct){
171
+
b: (struct){
172
+
}
173
+
}
174
+
}
175
+
}
176
+
a2: (struct){
177
+
out: (struct){
178
+
b: (struct){
179
+
Val: (int){ 1 }
180
+
}
181
+
}
182
+
in: (struct){
183
+
a: (struct){
184
+
b: (struct){
185
+
}
186
+
}
187
+
}
188
+
}
189
+
a3: (struct){
190
+
in: (struct){
191
+
a: (struct){
192
+
b: (struct){
193
+
}
194
+
}
195
+
}
196
+
out: (struct){
197
+
b: (struct){
198
+
Val: (int){ 1 }
199
+
}
200
+
}
201
+
}
202
+
a4: (struct){
203
+
out: (struct){
204
+
b: (struct){
205
+
Val: (int){ 1 }
206
+
}
207
+
}
208
+
in: (struct){
209
+
a: (struct){
210
+
b: (struct){
211
+
}
212
+
}
213
+
}
214
+
}
215
+
a5: (struct){
216
+
out: (struct){
217
+
b: (struct){
218
+
Val: (int){ 1 }
219
+
}
220
+
}
221
+
in: (struct){
222
+
a: (struct){
223
+
b: (struct){
224
+
}
225
+
}
226
+
}
227
+
}
228
+
a6: (struct){
229
+
in: (struct){
230
+
a: (struct){
231
+
b: (struct){
232
+
}
233
+
}
234
+
}
235
+
out: (struct){
236
+
b: (struct){
237
+
Val: (int){ 1 }
238
+
}
239
+
}
240
+
}
241
+
}
242
+
issue3691: (_|_){
243
+
// [eval]
244
+
original: (_|_){
245
+
// [eval]
246
+
X: (struct){
247
+
}
248
+
a: (_|_){
249
+
// [eval]
250
+
b: (_|_){
251
+
// [eval]
252
+
c: (_|_){
253
+
// [eval] issue3691.original.a.b: conflicting values 1 and string (mismatched types int and string):
254
+
// ./issue3691.cue:4:15
255
+
// ./issue3691.cue:8:10
256
+
}
257
+
}
258
+
}
259
+
}
260
+
structuralCycle: (_|_){
261
+
// [structural cycle]
262
+
X: (struct){
263
+
}
264
+
a: (_|_){
265
+
// [structural cycle]
266
+
b: (_|_){
267
+
// [structural cycle]
268
+
c: (_|_){
269
+
// [structural cycle] issue3691.structuralCycle.a.b.c: structural cycle
270
+
}
271
+
}
272
+
}
273
+
}
274
+
}
275
+
}
276
+
-- diff/-out/evalalpha<==>+out/eval --
277
+
diff old new
278
+
--- old
279
+
+++ new
280
+
@@ -1,14 +1,8 @@
281
+
Errors:
282
+
-issue3691.original.a.b.c: conflicting values string and 1 (mismatched types string and int):
283
+
+issue3691.original.a.b: conflicting values 1 and string (mismatched types int and string):
284
+
./issue3691.cue:4:15
285
+
- ./issue3691.cue:7:3
286
+
./issue3691.cue:8:10
287
+
-issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
288
+
- ./issue3691.cue:14:15
289
+
- ./issue3691.cue:15:5
290
+
- ./issue3691.cue:17:3
291
+
- ./issue3691.cue:18:10
292
+
-issue3691.structuralCycle.a.b.c.b.c: structural cycle
293
+
+issue3691.structuralCycle.a.b.c: structural cycle
294
+
295
+
Result:
296
+
(_|_){
297
+
@@ -125,9 +119,8 @@
298
+
b: (_|_){
299
+
// [eval]
300
+
c: (_|_){
301
+
- // [eval] issue3691.original.a.b.c: conflicting values string and 1 (mismatched types string and int):
302
+
+ // [eval] issue3691.original.a.b: conflicting values 1 and string (mismatched types int and string):
303
+
// ./issue3691.cue:4:15
304
+
- // ./issue3691.cue:7:3
305
+
// ./issue3691.cue:8:10
306
+
}
307
+
}
308
+
@@ -134,25 +127,15 @@
309
+
}
310
+
}
311
+
structuralCycle: (_|_){
312
+
- // [eval]
313
+
- X: (struct){
314
+
- }
315
+
- a: (_|_){
316
+
- // [eval]
317
+
- b: (_|_){
318
+
- // [eval]
319
+
- c: (_|_){
320
+
- // [eval] issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
321
+
- // ./issue3691.cue:14:15
322
+
- // ./issue3691.cue:15:5
323
+
- // ./issue3691.cue:17:3
324
+
- // ./issue3691.cue:18:10
325
+
- b: (_|_){
326
+
- // [structural cycle]
327
+
- c: (_|_){
328
+
- // [structural cycle] issue3691.structuralCycle.a.b.c.b.c: structural cycle
329
+
- }
330
+
- }
331
+
+ // [structural cycle]
332
+
+ X: (struct){
333
+
+ }
334
+
+ a: (_|_){
335
+
+ // [structural cycle]
336
+
+ b: (_|_){
337
+
+ // [structural cycle]
338
+
+ c: (_|_){
339
+
+ // [structural cycle] issue3691.structuralCycle.a.b.c: structural cycle
340
+
}
341
+
}
342
+
}
343
+
-- diff/todo/p2 --
344
+
Missing error positions.
110
345
-- out/eval --
111
-
(struct){
346
+
Errors:
347
+
issue3691.original.a.b.c: conflicting values string and 1 (mismatched types string and int):
348
+
./issue3691.cue:4:15
349
+
./issue3691.cue:7:3
350
+
./issue3691.cue:8:10
351
+
issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
352
+
./issue3691.cue:14:15
353
+
./issue3691.cue:15:5
354
+
./issue3691.cue:17:3
355
+
./issue3691.cue:18:10
356
+
issue3691.structuralCycle.a.b.c.b.c: structural cycle
357
+
358
+
Result:
359
+
(_|_){
360
+
// [eval]
112
361
a: (struct){
113
362
x: (int){ 10 }
114
363
y: (int){ 100 }
···
210
459
}
211
460
}
212
461
}
462
+
issue3691: (_|_){
463
+
// [eval]
464
+
original: (_|_){
465
+
// [eval]
466
+
X: (struct){
467
+
}
468
+
a: (_|_){
469
+
// [eval]
470
+
b: (_|_){
471
+
// [eval]
472
+
c: (_|_){
473
+
// [eval] issue3691.original.a.b.c: conflicting values string and 1 (mismatched types string and int):
474
+
// ./issue3691.cue:4:15
475
+
// ./issue3691.cue:7:3
476
+
// ./issue3691.cue:8:10
477
+
}
478
+
}
479
+
}
480
+
}
481
+
structuralCycle: (_|_){
482
+
// [eval]
483
+
X: (struct){
484
+
}
485
+
a: (_|_){
486
+
// [eval]
487
+
b: (_|_){
488
+
// [eval]
489
+
c: (_|_){
490
+
// [eval] issue3691.structuralCycle.a.b.c: conflicting values string and {[string]:X} (mismatched types string and struct):
491
+
// ./issue3691.cue:14:15
492
+
// ./issue3691.cue:15:5
493
+
// ./issue3691.cue:17:3
494
+
// ./issue3691.cue:18:10
495
+
b: (_|_){
496
+
// [structural cycle]
497
+
c: (_|_){
498
+
// [structural cycle] issue3691.structuralCycle.a.b.c.b.c: structural cycle
499
+
}
500
+
}
501
+
}
502
+
}
503
+
}
504
+
}
505
+
}
213
506
}
214
507
-- out/compile --
215
508
--- in.cue
···
345
638
}
346
639
}
347
640
}
641
+
--- issue3691.cue
642
+
{
643
+
issue3691: {
644
+
original: {
645
+
X: {
646
+
[string]: string
647
+
}
648
+
a: {
649
+
[string]: 〈1;X〉
650
+
}
651
+
a: {
652
+
if true {
653
+
b: {
654
+
c: 1
655
+
}
656
+
}
657
+
}
658
+
}
659
+
}
660
+
issue3691: {
661
+
structuralCycle: {
662
+
X: {
663
+
[string]: string
664
+
}
665
+
a: {
666
+
[string]: 〈1;X〉
667
+
}
668
+
a: {
669
+
if true {
670
+
b: {
671
+
c: 〈3;a〉
672
+
}
673
+
}
674
+
}
675
+
}
676
+
}
677
+
}
+9
-1
internal/core/adt/dep.go
+9
-1
internal/core/adt/dep.go
···
337
337
338
338
// breakIncomingDeps breaks all incoming dependencies, which includes arcs and
339
339
// pending notifications and attempts all remaining work.
340
-
func (n *nodeContext) breakIncomingDeps() {
340
+
//
341
+
// We should only break incoming dependencies if we are finalizing nodes, as
342
+
// breaking them earlier can cause a "already closed" panic. To make sure of
343
+
// this, we force the caller to pass mode.
344
+
func (n *nodeContext) breakIncomingDeps(mode runMode) {
345
+
if mode != finalize {
346
+
return
347
+
}
348
+
341
349
// TODO: remove this block in favor of finalizing notification nodes,
342
350
// or what have you. We have patched this to skip evaluating when using
343
351
// disjunctions, but this is overall a brittle approach.
+2
-2
internal/core/adt/unify.go
+2
-2
internal/core/adt/unify.go
···
285
285
case needs&subFieldsProcessed != 0:
286
286
switch {
287
287
case assertStructuralCycleV3(n):
288
-
n.breakIncomingDeps()
288
+
n.breakIncomingDeps(mode)
289
289
// TODO: consider bailing on error if n.errs != nil.
290
290
case n.completeAllArcs(needs, mode):
291
291
}
···
539
539
// Investigate how to work around this.
540
540
n.completeNodeTasks(finalize)
541
541
542
-
n.breakIncomingDeps()
542
+
n.breakIncomingDeps(mode)
543
543
544
544
n.incDepth()
545
545
defer n.decDepth()