mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

internal/core/export: fix panic in relPathLength for BinaryExpr

Extracting a sub-expression with nested (Def & {...}).field
patterns across package boundaries panics in relPathLength
with "unreachable" because SelectorExpr.X can be a BinaryExpr,
which is not handled.

We need to apply two fixes to resolve the panic:

1. In relPathLength, a BinaryExpr (or any non-Resolver) as the base of
a selector chain is a valid end-of-chain. Return (length, false)
instead of panicking.

2. In completePivot, use a fixpoint loop instead of a single pass.
Exporting one external's body (in addExternal) may set needTopLevel
on deps that appear earlier in the list. Without the fixpoint loop,
those deps are never hoisted as let bindings.

Note that the added testdata file reproduces the panic without the fix,
so we must add the test alongside the fix in one commit.

Fixes #4237.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ie93ad75a3504c633c92f9d9315e77ab7ca09e89f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1231150
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

Daniel Martí 788eeed1 08a7ec36

+85 -5
+18 -5
internal/core/export/self.go
··· 56 56 if s == nil || f == nil { 57 57 return 58 58 } 59 - for _, d := range s.deps { 60 - if !d.isExternalRoot() { 61 - continue 59 + // Use a fixpoint loop because exporting one external's body (in 60 + // addExternal) may set needTopLevel on deps earlier in the list. 61 + for { 62 + progress := false 63 + for _, d := range s.deps { 64 + if d.exported || !d.isExternalRoot() || !d.needTopLevel { 65 + continue 66 + } 67 + d.exported = true 68 + progress = true 69 + s.addExternal(d) 62 70 } 63 - s.addExternal(d) 71 + if !progress { 72 + break 73 + } 64 74 } 65 75 f.Decls = append(f.Decls, s.decls...) 66 76 } ··· 95 105 useCount int // Other reference using this vertex 96 106 included bool 97 107 needTopLevel bool 108 + exported bool // already processed by addExternal in the fixpoint loop 98 109 } 99 110 100 111 // isExternalRoot reports whether d is an external node (a node referenced ··· 385 396 case adt.Resolver: 386 397 r = x 387 398 default: 388 - panic("unreachable") 399 + // A non-Resolver expression (e.g. BinaryExpr) is a valid 400 + // end-of-chain; we simply cannot walk any further. 401 + return length, false 389 402 } 390 403 } 391 404 }
+67
internal/core/export/testdata/selfcontained/issue4237.txtar
··· 1 + #path: out 2 + 3 + Extracting a sub-expression that contains nested (Def & {...}).field 4 + expressions used to cause a panic. 5 + 6 + -- cue.mod/module.cue -- 7 + module: "test.example" 8 + language: version: "v0.9.0" 9 + -- in.cue -- 10 + import "test.example/pkg" 11 + 12 + pkg.#Module 13 + -- pkg/pkg.cue -- 14 + package pkg 15 + 16 + #B: { 17 + out: {x: 1} 18 + } 19 + 20 + #A: { 21 + out: (#B & {}).out 22 + } 23 + 24 + #Module: { 25 + out: (#A & {}).out 26 + } 27 + -- out/self-v3/default -- 28 + 29 + _#def 30 + _#def: (A.#x & {}).out 31 + 32 + //cue:path: #A 33 + let A = { 34 + #x: { 35 + out: (B.#x & {}).out 36 + } 37 + } 38 + 39 + //cue:path: #B 40 + let B = { 41 + #x: { 42 + out: { 43 + x: 1 44 + } 45 + } 46 + } 47 + -- out/self-v3-noshare/default -- 48 + 49 + _#def 50 + _#def: (A.#x & {}).out 51 + 52 + //cue:path: #A 53 + let A = { 54 + #x: { 55 + out: (B.#x & {}).out 56 + } 57 + } 58 + 59 + //cue:path: #B 60 + let B = { 61 + #x: { 62 + out: { 63 + x: 1 64 + } 65 + } 66 + } 67 + -- out/self/default --