Skip to content

Commit bc6c485

Browse files
cuematthewcuematthew
committed
core/toposort: stop trying to order structinfos/structlits
A Vertex has a Structs field which contains StructInfos for all the structs that contributed to this vertex. Previously, I was attempting to build a graph from these StructInfos. This necessitated storing the parent of a struct in its StructInfo. However, even if this is working perfectly, the results are not perfect. For example: ``` y: b: 2 x: { c: 1 y a: 1 } ``` Here, the Vertex for `x` would contain two StructInfos, but we still wouldn't be able to establish that the `{b: 2}` struct should be processed after `c: 1` and before `a: 1`. This is because the vertex for `x` does not know there was an embedding. However, here: ``` x: { c: 1 if true { b: 2 } a: 1 } ``` on master, this extra parent info would allow us to get the ordering right, (i.e. c, then b, then a). This is because we would discover that `{b: 2}`'s ancestor is `x`, This CL removes this extra parent information completely. This means the above two examples now behave the same (b, then c, then a), which is wrong, but it's more consistent. It also simplifies the code substantially. It also makes the behaviour of evalv3 closer to evalv2. Signed-off-by: Matthew Sackman <[email protected]> Change-Id: I973230e591894b4b4018b66e2967f65339d1026a Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211097 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Marcel van Lohuizen <[email protected]>
1 parent 5d2da07 commit bc6c485

26 files changed

+60
-400
lines changed

‎.github/workflows/evict_caches.yaml

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ jobs:
2121
run: touch -t 202211302355 $(find * -type d)
2222
- name: Restore git file modification times
2323
uses: chetan/git-restore-mtime-action@075f9bc9d159805603419d50f794bd9f33252ebe
24-
- name: Try to extract Dispatch-Trailer
25-
id: DispatchTrailer
24+
- id: DispatchTrailer
25+
name: Try to extract Dispatch-Trailer
2626
run: |-
2727
x="$(git log -1 --pretty='%(trailers:key=Dispatch-Trailer,valueonly)')"
2828
if [[ "$x" == "" ]]
@@ -38,11 +38,11 @@ jobs:
3838
echo "value<<EOD" >> $GITHUB_OUTPUT
3939
echo "$x" >> $GITHUB_OUTPUT
4040
echo "EOD" >> $GITHUB_OUTPUT
41-
- name: Check we don't have Dispatch-Trailer on a protected branch
42-
if: |-
41+
- if: |-
4342
((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
4443
Dispatch-Trailer: {"type":"')))) && (contains(github.event.head_commit.message, '
4544
Dispatch-Trailer: {"type":"'))
45+
name: Check we don't have Dispatch-Trailer on a protected branch
4646
run: |-
4747
echo "github.event.head_commit.message contains Dispatch-Trailer but we are on a protected branch"
4848
false

‎.github/workflows/trybot.yaml

+3-3
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,12 @@ jobs:
140140
Dispatch-Trailer: {"type":"')))) || (github.ref == 'refs/heads/ci/test')) && (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
141141
name: gcloud setup for end-to-end tests
142142
uses: google-github-actions/setup-gcloud@v2
143-
- env:
144-
CUE_TEST_TOKEN: ${{ secrets.E2E_PORCUEPINE_CUE_TOKEN }}
145-
if: |-
143+
- if: |-
146144
github.repository == 'cue-lang/cue' && (((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
147145
Dispatch-Trailer: {"type":"')))) || (github.ref == 'refs/heads/ci/test')) && (matrix.go-version == '1.24.x' && matrix.runner == 'ubuntu-24.04')
148146
name: End-to-end test
147+
env:
148+
CUE_TEST_TOKEN: ${{ secrets.E2E_PORCUEPINE_CUE_TOKEN }}
149149
run: |-
150150
cd internal/_e2e
151151
go test -race

‎cmd/cue/cmd/testdata/script/cmd_issue2060.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ StrokesName: ZenStrokes1
136136
PlatformName: Linux
137137
CommandBindingsCount: 1
138138
CommandBindings:
139-
- BindingsMax: 0
139+
- EditorCmdText: '"command": "move", "args": test'
140+
BindingsMax: 0
140141
CmdName: CursorMoveToLineForward
141142
CmdHumanName: Cursor Down
142-
EditorCmdText: '"command": "move", "args": test'
143143
BindingsCount: 1
144144
Bindings:
145145
- DefText: Down

‎cmd/cue/cmd/testdata/script/issue269.txtar

+4-4
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ data: {
2727
}
2828
data: {
2929
a: {
30-
x: 0
31-
y: 0
3230
i: 0
3331
j: 0
34-
}
35-
b: {
3632
x: 0
3733
y: 0
34+
}
35+
b: {
3836
i: 0
3937
j: 0
38+
x: 0
39+
y: 0
4040
}
4141
}

‎cmd/cue/cmd/testdata/script/issue986.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ cmp stdout expect-stdout
3131

3232
-- expect-stdout --
3333
a: aaa
34-
ab: aaabbb
3534
b: bbb
35+
ab: aaabbb
3636
cd: cccddd

‎cmd/cue/cmd/testdata/script/modget_initial.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ deps: {
6565
}
6666
-- want-stdout-1 --
6767
{
68-
"foo.com/bar/hello@v0": "v0.2.3",
6968
"bar.com@v0": "v0.5.0",
7069
"baz.org@v0": "v0.10.2",
70+
"foo.com/bar/hello@v0": "v0.2.3",
7171
"main": "main",
7272
"example.com@v0": "v0.0.1"
7373
}
@@ -93,9 +93,9 @@ deps: {
9393
}
9494
-- want-stdout-2 --
9595
{
96-
"foo.com/bar/hello@v0": "v0.2.3",
9796
"bar.com@v0": "v0.5.0",
9897
"baz.org@v0": "v0.11.0-alpha",
98+
"foo.com/bar/hello@v0": "v0.2.3",
9999
"main": "main",
100100
"example.com@v0": "v0.0.1"
101101
}

‎cmd/cue/cmd/testdata/script/modmirror_initial.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ mirroring foo.com/bar/[email protected]
3535
mirroring [email protected]
3636
-- want-export-stdout --
3737
{
38-
"foo.com/bar/hello@v0": "v0.2.3",
3938
"bar.com@v0": "v0.5.0",
4039
"baz.org@v0": "v0.10.1",
40+
"foo.com/bar/hello@v0": "v0.2.3",
4141
"main": "main",
4242
"example.com@v0": "v0.0.1"
4343
}

‎cmd/cue/cmd/testdata/script/modmirror_modmode.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ mirroring [email protected]
3535
mirroring foo.com/bar/[email protected]
3636
-- want-export-stdout --
3737
{
38-
"foo.com/bar/hello@v0": "v0.2.3",
3938
"bar.com@v0": "v0.5.0",
4039
"baz.org@v0": "v0.10.1",
40+
"foo.com/bar/hello@v0": "v0.2.3",
4141
"main": "main",
4242
"example.com@v0": "v0.0.1"
4343
}

‎cmd/cue/cmd/testdata/script/modmirror_nodeps.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ mirroring [email protected]
2323
failed to resolve "foo.com/bar/hello": cannot find module providing package foo.com/bar/hello
2424
-- want-export-stdout --
2525
{
26-
"foo.com/bar/hello@v0": "v0.2.3",
2726
"bar.com@v0": "v0.5.0",
2827
"baz.org@v0": "v0.10.1",
28+
"foo.com/bar/hello@v0": "v0.2.3",
2929
"main": "main",
3030
"example.com@v0": "v0.0.1"
3131
}

‎cmd/cue/cmd/testdata/script/modtidy_initial.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ deps: {
4040
}
4141
-- want-stdout --
4242
{
43-
"foo.com/bar/hello@v0": "v0.2.3",
4443
"bar.com@v0": "v0.5.0",
4544
"baz.org@v0": "v0.10.1",
45+
"foo.com/bar/hello@v0": "v0.2.3",
4646
"main": "main",
4747
"example.com@v0": "v0.0.1"
4848
}

‎cmd/cue/cmd/testdata/script/registry_mux.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ cmp stdout expect-stdout
2323
defaultRegistry: registry: "${CUE_REGISTRY1}"
2424
moduleRegistries: "baz.org": registry: "${CUE_REGISTRY2}"
2525
-- expect-stdout --
26-
"foo.com/bar/hello@v0": "v0.2.3"
2726
"bar.com@v0": "v0.5.0"
2827
"baz.org@v0": "v0.10.1 in registry2"
28+
"foo.com/bar/hello@v0": "v0.2.3"
2929
main: "main"
3030
"example.com@v0": "v0.0.1"
3131
-- cue.mod/module.cue --

‎cmd/cue/cmd/testdata/script/registry_mux_auth.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ env-fill $DOCKER_CONFIG/config.json
77
exec cue eval .
88
cmp stdout expect-stdout
99
-- expect-stdout --
10-
"foo.com/bar/hello@v0": "v0.2.3"
1110
"bar.com@v0": "v0.5.0"
1211
"baz.org@v0": "v0.10.1 in registry2"
12+
"foo.com/bar/hello@v0": "v0.2.3"
1313
main: "main"
1414
"example.com@v0": "v0.0.1"
1515
-- dockerconfig/config.json --

‎cmd/cue/cmd/testdata/script/registry_publish.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ env CUE_CACHE_DIR=$WORK/.tmp/different-cache
1616
stderr 'cannot fetch [email protected]: module [email protected]: module not found'
1717

1818
-- expect-eval-stdout --
19-
"foo.com/bar/hello@v0": "v0.2.3"
2019
"bar.com@v0": "v0.5.0"
2120
"baz.org@v0": "v0.10.1"
21+
"foo.com/bar/hello@v0": "v0.2.3"
2222
main: "main"
2323
"example.com@v0": "v0.0.1"
2424
-- main/cue.mod/module.cue --

‎cmd/cue/cmd/testdata/script/registry_publish_with_git.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ VCS state is not clean
115115
-- expect-eval-stdout2 --
116116
root: true
117117
a: true
118-
sensitive_a: true
119118
b: true
119+
sensitive_a: true
120120
-- main/cue.mod/module.cue --
121121
module: "main.org@v0"
122122
language: version: "v0.9.0-alpha.0"

‎cmd/cue/cmd/testdata/script/registry_simple.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,16 @@ exec cue trim .
99
exec cue vet .
1010
exec cue fmt .
1111
-- expect-stdout --
12-
"foo.com/bar/hello@v0": "v0.2.3"
1312
"bar.com@v0": "v0.5.0"
1413
"baz.org@v0": "v0.10.1"
14+
"foo.com/bar/hello@v0": "v0.2.3"
1515
main: "main"
1616
"example.com@v0": "v0.0.1"
1717
-- expect-stdout-json --
1818
{
19-
"foo.com/bar/hello@v0": "v0.2.3",
2019
"bar.com@v0": "v0.5.0",
2120
"baz.org@v0": "v0.10.1",
21+
"foo.com/bar/hello@v0": "v0.2.3",
2222
"main": "main",
2323
"example.com@v0": "v0.0.1"
2424
}

‎cue/load/testdata/testfetch/simple.txtar

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
-- out/modfetch --
22
{
3-
"foo.com/bar/hello@v0": "v0.2.3"
43
"bar.com@v0": "v0.5.0"
54
"baz.org@v0": "v0.10.1"
5+
"foo.com/bar/hello@v0": "v0.2.3"
66
main: "main"
77
"example.com@v0": "v0.0.1"
88
}

‎internal/core/adt/closed.go

-4
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,6 @@ type closeInfo struct {
291291

292292
root SpanType
293293
span SpanType
294-
295-
// decl is the parent declaration which contains the conjuct which
296-
// gave rise to this closeInfo.
297-
decl Decl
298294
}
299295

300296
// closeStats holds the administrative fields for a closeInfo value. Each

‎internal/core/adt/composite.go

-14
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,6 @@ func (v *Vertex) rootCloseContext(ctx *OpContext) *closeContext {
330330
parent: nil,
331331
src: v,
332332
parentConjuncts: v,
333-
decl: v,
334333
arcType: mode,
335334
}
336335
v._cc.incDependent(ctx, ROOT, nil) // matched in REF(decrement:nodeDone)
@@ -575,9 +574,6 @@ type StructInfo struct {
575574
Disable bool
576575

577576
Embedding bool
578-
579-
// Decl contains this Struct
580-
Decl Decl
581577
}
582578

583579
// TODO(perf): this could be much more aggressive for eliminating structs that
@@ -1540,16 +1536,6 @@ func (v *Vertex) AddStruct(s *StructLit, env *Environment, ci CloseInfo) *Struct
15401536
Env: env,
15411537
CloseInfo: ci,
15421538
}
1543-
if env.Vertex != nil {
1544-
// be careful to avoid promotion of nil env.Vertex to non-nil
1545-
// info.Decl
1546-
info.Decl = env.Vertex
1547-
}
1548-
if cc := ci.cc; cc != nil && cc.decl != nil {
1549-
info.Decl = cc.decl
1550-
} else if ci := ci.closeInfo; ci != nil && ci.decl != nil {
1551-
info.Decl = ci.decl
1552-
}
15531539
for _, t := range v.Structs {
15541540
if *t == info { // TODO: check for different identity.
15551541
return t

‎internal/core/adt/comprehension.go

-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ func (n *nodeContext) insertComprehension(
160160
if !n.ctx.isDevVersion() {
161161
ci = ci.SpawnEmbed(c)
162162
ci.closeInfo.span |= ComprehensionSpan
163-
ci.decl = c
164163
}
165164

166165
var decls []Decl

‎internal/core/adt/conjunct.go

-2
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,6 @@ loop1:
346346

347347
case *Comprehension:
348348
ci, cc := ci.spawnCloseContext(n.ctx, closeEmbed)
349-
cc.decl = x
350349
cc.incDependent(n.ctx, DEFER, nil)
351350
defer cc.decDependent(n.ctx, DEFER, nil)
352351
n.insertComprehension(childEnv, x, ci)
@@ -373,7 +372,6 @@ loop1:
373372
case Expr:
374373
// TODO: perhaps special case scalar Values to avoid creating embedding.
375374
ci, cc := ci.spawnCloseContext(n.ctx, closeEmbed)
376-
cc.decl = x
377375

378376
// TODO: do we need to increment here?
379377
cc.incDependent(n.ctx, DEFER, nil) // decrement deferred below

‎internal/core/adt/eval.go

-1
Original file line numberDiff line numberDiff line change
@@ -2227,7 +2227,6 @@ func (n *nodeContext) addStruct(
22272227
// TODO(perf): only do this if addExprConjunct below will result in
22282228
// a fieldSet. Otherwise the entry will just be removed next.
22292229
id := closeInfo.SpawnEmbed(x)
2230-
id.decl = x
22312230

22322231
c := MakeConjunct(childEnv, x, id)
22332232
n.addExprConjunct(c, partial)

‎internal/core/export/testdata/main/adt.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ _|_ // e3: index out of range [2] with length 2
448448
== All
449449
{
450450
@foo(bar)
451+
x: int
451452
p1: {}
452453
d1: {
453454
foobar: int
@@ -512,14 +513,13 @@ _|_ // e3: index out of range [2] with length 2
512513
// baz is a required field.
513514
baz!: 5
514515
}
515-
x: int
516516
y1: {
517-
src: [1, 2, 3]
518517
foo0: 1
519518
bar1: 2
520519
bar2: 3
521520
foo1: 2
522521
foo2: 3
522+
src: [1, 2, 3]
523523
x: [1, 2, 3]
524524
}
525525
preserveKeyFieldInComprehension: 1

0 commit comments

Comments
 (0)