this repo has no description
0
fork

Configure Feed

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

internal/core/export: avoid introducing shadowing

The solution is to unconditionally link Ident.Node fields with
their resolution, allowing the sanitizer to unshadow.
The exporter is becoming quite a mess and is due for an
overhaul. The implementation should be notably easier with the
upcoming evaluator overhaul, so it seems a waste to do that now.

Fixes #2311

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I38d2c010814e5d97424d72890da002bb1c6ef2cb
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/552252
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

+323 -84
+42 -1
internal/core/export/adt.go
··· 297 297 switch x := r.(type) { 298 298 case *adt.FieldReference: 299 299 ident, _ := e.newIdentForField(x.Src, x.Label, x.UpCount) 300 + 301 + // Use low-level lookup to bypass structural cycle detection. This is 302 + // fine as we do not recurse on the result and it is necessary to detect 303 + // shadowing even when a configuration has a structural cycle. 304 + for i := 0; i < int(x.UpCount); i++ { 305 + env = env.Up 306 + } 307 + 308 + // Exclude comprehensions and other temporary/ inlined Vertices, which 309 + // cannot be properly resolved, throwing off the sanitize. Also, 310 + // comprehensions originate from a single source and do not need to be 311 + // handled. 312 + if v := env.Vertex; !v.IsDynamic { 313 + if v = v.Lookup(x.Label); v != nil { 314 + e.linkIdentifier(v, ident) 315 + } 316 + } 317 + 300 318 return ident 301 319 302 320 case *adt.ValueReference: ··· 424 442 internal.SetConstraint(f, x.ArcType.Token()) 425 443 e.setField(x.Label, f) 426 444 427 - f.Value = e.expr(env, x.Value) 428 445 f.Attrs = extractFieldAttrs(nil, x) 446 + 447 + st, ok := x.Value.(*adt.StructLit) 448 + if !ok { 449 + f.Value = e.expr(env, x.Value) 450 + return f 451 + 452 + } 453 + 454 + top := e.frame(0) 455 + var src *adt.Vertex 456 + if top.node != nil { 457 + src = top.node.Lookup(x.Label) 458 + } 459 + 460 + // Instead of calling e.expr directly, we inline the case for 461 + // *adt.StructLit, so that we can pass src. 462 + c := adt.MakeRootConjunct(env, st) 463 + f.Value = e.mergeValues(adt.InvalidLabel, src, []conjunct{{c: c, up: 0}}, c) 464 + 465 + if top.node != nil { 466 + if v := top.node.Lookup(x.Label); v != nil { 467 + e.linkField(v, f) 468 + } 469 + } 429 470 430 471 return f 431 472
+52 -2
internal/core/export/export.go
··· 247 247 labelAlias map[adt.Expr]adt.Feature 248 248 valueAlias map[*ast.Alias]*ast.Alias 249 249 letAlias map[*ast.LetClause]*ast.LetClause 250 + references map[*adt.Vertex]*referenceInfo 250 251 251 252 usedHidden map[string]bool 252 253 253 254 pivotter *pivotter 254 255 } 255 256 257 + // referenceInfo is used to track which Field.Value fields should be linked 258 + // to Ident.Node fields. The Node field is used by astutil.Resolve to mark 259 + // the value in the AST to which the respective identifier points. 260 + // astutil.Sanitize, in turn, uses this information to determine whether 261 + // a reference is shadowed and apply fixes accordingly. 262 + type referenceInfo struct { 263 + field *ast.Field 264 + references []*ast.Ident 265 + } 266 + 267 + // linkField reports the Field that represents certain Vertex in the generated 268 + // output. The Node fields for any references (*ast.Ident) that were already 269 + // recorded as pointed to this vertex are updated accordingly. 270 + func (e *exporter) linkField(v *adt.Vertex, f *ast.Field) { 271 + if v == nil { 272 + return 273 + } 274 + refs := e.references[v] 275 + if refs == nil { 276 + // TODO(perf): do a first sweep to only mark referenced arcs or keep 277 + // track of that information elsewhere. 278 + e.references[v] = &referenceInfo{field: f} 279 + return 280 + } 281 + for _, r := range refs.references { 282 + r.Node = f.Value 283 + } 284 + refs.references = refs.references[:0] 285 + } 286 + 287 + // linkIdentifier reports the Vertex to which indent points. Once the ast.Field 288 + // for a corresponding Vertex is known, it is linked to ident. 289 + func (e *exporter) linkIdentifier(v *adt.Vertex, ident *ast.Ident) { 290 + refs := e.references[v] 291 + if refs == nil { 292 + refs = &referenceInfo{} 293 + e.references[v] = refs 294 + } 295 + if refs.field == nil { 296 + refs.references = append(refs.references, ident) 297 + return 298 + } 299 + ident.Node = refs.field.Value 300 + } 301 + 256 302 // newExporter creates and initializes an exporter. 257 303 func newExporter(p *Profile, r adt.Runtime, pkgID string, v adt.Value) *exporter { 258 304 n, _ := v.(*adt.Vertex) ··· 261 307 ctx: eval.NewContext(r, n), 262 308 index: r, 263 309 pkgID: pkgID, 310 + 311 + references: map[*adt.Vertex]*referenceInfo{}, 264 312 } 265 313 266 314 e.markUsedFeatures(v) ··· 606 654 setFieldAlias(f.field, f.alias) 607 655 node = f.field 608 656 } 609 - for _, r := range f.references { 610 - r.Node = node 657 + if node != nil { 658 + for _, r := range f.references { 659 + r.Node = node 660 + } 611 661 } 612 662 } 613 663
+2
internal/core/export/expr.go
··· 259 259 260 260 d.Value = e.mergeValues(f, field.arc, c, a...) 261 261 262 + e.linkField(field.arc, d) 263 + 262 264 if f.IsDef() { 263 265 x.inDefinition-- 264 266 }
+227 -81
internal/core/export/testdata/main/shadow.txtar
··· 1 - In this file we test cases where a newly created dynamic field 2 - ends up shadowing an existing field. 1 + In this file we test cases where references are shadowed: 2 + - x: x: x 3 + - a newly created dynamic field which ends up shadowing an existing field. 3 4 4 5 -- in.cue -- 5 - y: "outer" 6 - a: { 7 - key: "y" 8 - X=(key): "inner" 6 + shadowRef: t1: { 7 + x: "x": x | null 8 + } 9 9 10 - refs: { 11 - Z="y": 1 12 - outer: y 13 - mid: X // TODO(unshadow) 14 - inner: Z // TODO(unshadow) 10 + shadowRef: t2: { 11 + x: "x": "x": x | null 12 + } 13 + 14 + shadowRef: t3: { 15 + x: "x": x: "x": x | null 16 + } 15 17 18 + shadowRef: t4: { 19 + X=x: { 20 + x: X | null 21 + y: x 16 22 } 17 23 } 18 - -- out/definition -- 19 - y: "outer" 20 - a: { 21 - key: "y" 22 - X=(key): "inner" 23 - refs: { 24 - y: 1 25 - outer: y 26 - mid: X 27 - inner: y 28 - } 24 + 25 + shadowRef: t5: { 26 + X=x: x: X | null 27 + } 28 + 29 + shadowRef: e1: { 30 + x: "x": x // structural error, but should still print ok. 31 + x: {} 29 32 } 30 - -- out/doc -- 31 - [] 32 - [y] 33 - [a] 34 - [a key] 35 - [a refs] 36 - [a refs y] 37 - [a refs outer] 38 - [a refs mid] 39 - [a refs inner] 40 - [a y] 41 - -- out/value -- 42 - == Simplified 43 - { 33 + 34 + shadowAlias: t1: { 44 35 y: "outer" 45 36 a: { 46 37 key: "y" 47 - y: "inner" 38 + X=(key): "inner" 39 + 48 40 refs: { 49 - y: 1 50 - outer: "outer" 51 - mid: "inner" 52 - inner: 1 41 + Z="y": 1 42 + outer: y 43 + mid: X 44 + inner: Z 45 + 53 46 } 54 47 } 55 48 } 56 - == Raw 57 - { 58 - y: "outer" 49 + 50 + shadowAlias: t2: { 59 51 a: { 60 52 key: "y" 61 - y: "inner" 53 + X=(key): "inner" 54 + 62 55 refs: { 63 - y: 1 64 - outer: "outer" 65 - mid: "inner" 66 - inner: 1 56 + Z="y": 1 57 + outer: y 58 + mid: X 59 + inner: Z 60 + 67 61 } 68 62 } 63 + y: "outer" 69 64 } 70 - == Final 71 - { 72 - y: "outer" 73 - a: { 74 - key: "y" 75 - y: "inner" 76 - refs: { 77 - y: 1 78 - outer: "outer" 79 - mid: "inner" 80 - inner: 1 65 + -- out/definition -- 66 + shadowRef: { 67 + t1: { 68 + x_1=x: { 69 + x: x_1 | null 70 + } 71 + } 72 + t2: { 73 + x_5=x: { 74 + x: { 75 + x: x_5 | null 76 + } 77 + } 78 + } 79 + t3: { 80 + x: { 81 + x: { 82 + x_A=x: { 83 + x: x_A | null 84 + } 85 + } 86 + } 87 + } 88 + t4: { 89 + x_8=x: { 90 + x: x_8 | null 91 + y: x 92 + } 93 + } 94 + t5: { 95 + x_E=x: { 96 + x: x_E | null 97 + } 98 + } 99 + e1: { 100 + x_B=x: { 101 + x: x_B 81 102 } 82 103 } 83 104 } 84 - == All 85 - { 86 - y: "outer" 87 - a: { 88 - key: "y" 89 - y: "inner" 90 - refs: { 91 - y: 1 92 - outer: "outer" 93 - mid: "inner" 94 - inner: 1 105 + shadowAlias: { 106 + t1: { 107 + y_B=y: "outer" 108 + a: { 109 + key: "y" 110 + X=(key): "inner" 111 + refs: { 112 + y: 1 113 + outer: y_B 114 + mid: X 115 + inner: y 116 + } 95 117 } 96 118 } 119 + t2: { 120 + a: { 121 + key: "y" 122 + X_1=(key): "inner" 123 + refs: { 124 + y: 1 125 + outer: y_6 126 + mid: X_1 127 + inner: y 128 + } 129 + } 130 + y_6=y: "outer" 131 + } 97 132 } 98 - == Eval 133 + -- out/doc -- 134 + [] 135 + [shadowRef] 136 + [shadowRef t1] 137 + [shadowRef t1 x] 138 + [shadowRef t1 x x] 139 + [shadowRef t2] 140 + [shadowRef t2 x] 141 + [shadowRef t2 x x] 142 + [shadowRef t2 x x x] 143 + [shadowRef t3] 144 + [shadowRef t3 x] 145 + [shadowRef t3 x x] 146 + [shadowRef t3 x x x] 147 + [shadowRef t3 x x x x] 148 + [shadowRef t4] 149 + [shadowRef t4 x] 150 + [shadowRef t4 x x] 151 + [shadowRef t4 x y] 152 + [shadowRef t5] 153 + [shadowRef t5 x] 154 + [shadowRef t5 x x] 155 + [shadowRef e1] 156 + [shadowRef e1 x] 157 + [shadowRef e1 x x] 158 + [shadowAlias] 159 + [shadowAlias t1] 160 + [shadowAlias t1 y] 161 + [shadowAlias t1 a] 162 + [shadowAlias t1 a key] 163 + [shadowAlias t1 a refs] 164 + [shadowAlias t1 a refs y] 165 + [shadowAlias t1 a refs outer] 166 + [shadowAlias t1 a refs mid] 167 + [shadowAlias t1 a refs inner] 168 + [shadowAlias t1 a y] 169 + [shadowAlias t2] 170 + [shadowAlias t2 a] 171 + [shadowAlias t2 a key] 172 + [shadowAlias t2 a refs] 173 + [shadowAlias t2 a refs y] 174 + [shadowAlias t2 a refs outer] 175 + [shadowAlias t2 a refs mid] 176 + [shadowAlias t2 a refs inner] 177 + [shadowAlias t2 a y] 178 + [shadowAlias t2 y] 179 + -- out/value -- 180 + == Simplified 181 + _|_ // shadowRef.e1.x.x: structural cycle (and 1 more errors) 182 + == Raw 183 + _|_ // shadowRef.e1.x.x: structural cycle (and 1 more errors) 184 + == Final 185 + _|_ // shadowRef.e1.x.x: structural cycle (and 1 more errors) 186 + == All 99 187 { 100 - y: "outer" 101 - a: { 102 - key: "y" 103 - y: "inner" 104 - refs: { 105 - y: 1 106 - outer: "outer" 107 - mid: "inner" 108 - inner: 1 188 + shadowRef: { 189 + t1: { 190 + x: { 191 + x: null 192 + } 193 + } 194 + t2: { 195 + x: { 196 + x: { 197 + x: null 198 + } 199 + } 200 + } 201 + t3: { 202 + x: { 203 + x: { 204 + x: { 205 + x: null 206 + } 207 + } 208 + } 209 + } 210 + t4: { 211 + x: { 212 + x: null 213 + y: null 214 + } 215 + } 216 + t5: { 217 + x: { 218 + x: null 219 + } 220 + } 221 + e1: { 222 + x: { 223 + x: _|_ // shadowRef.e1.x.x: structural cycle (and 1 more errors) 224 + } 225 + } 226 + } 227 + shadowAlias: { 228 + t1: { 229 + y: "outer" 230 + a: { 231 + key: "y" 232 + y: "inner" 233 + refs: { 234 + y: 1 235 + outer: "outer" 236 + mid: "inner" 237 + inner: 1 238 + } 239 + } 240 + } 241 + t2: { 242 + a: { 243 + key: "y" 244 + y: "inner" 245 + refs: { 246 + y: 1 247 + outer: "outer" 248 + mid: "inner" 249 + inner: 1 250 + } 251 + } 252 + y: "outer" 109 253 } 110 254 } 111 255 } 256 + == Eval 257 + _|_ // shadowRef.e1.x.x: structural cycle (and 1 more errors)