internal/core: __reclose: detect and restore non-recursive closing

+140 -5

cue/testdata/builtins/reclose.txtar

··· 55 55 z: ok: x & {d: f: int} @test(eq, {d: {f: int, e: int}, c: int, b: int}) 56 56 } 57 57 58 + // Test that __reclose detects and restores non-recursive closing from 59 + // an embedded close()-d value that was opened with .... 60 + nonRecursiveDetect: { 61 + foo: close({b: {}}) 62 + a: __reclose({ 63 + foo... 64 + }) 65 + // Top-level field not allowed: non-recursive close restored by __reclose. 66 + t1: err: a & {f: int} @test(err, code=eval, path=f, contains="field not allowed", pos=[0:16]) 67 + // Nested field allowed: close was non-recursive, so b is still open. 68 + t2: ok: a & {b: c: 1} @test(eq, {b: {c: 1}}) 69 + } 70 + 71 + // Test that __reclose handles both recursive and non-recursive closing 72 + // when both are present. 73 + // 74 + // NOTE: one would expect that with the old semantics, b.c is allowed in t2. 75 + // But it is NOT. To mimic the old behavior, cue fix _could_ just use __closeAll 76 + // in this case. Users can than chose to use __reclose instead to relax the 77 + // condition. 78 + mixedClose: { 79 + #D: {x: y: int} 80 + foo: close({b: {}}) 81 + a: __reclose({ 82 + #D... 83 + foo... 84 + }) 85 + // Top-level extra field not allowed. 86 + t1: err: a & {f: int} @test(err, code=eval, path=f, contains="field not allowed", pos=[0:16]) 87 + // Allowed fields work. 88 + t2: ok: a & {x: y: 5, b: c: 1} @test(eq, {x: {y: 5}, b: {c: 1}}) 89 + } 90 + 91 + // Test that __reclose can wrap a close() call and conditionally add 92 + // recursive closing on top. This is the pattern generated by cue fix 93 + // when a close() at field-value level wraps a struct with embeddings. 94 + recloseWrapsClose: { 95 + #def: {a: b: int} 96 + a: __reclose(close({ 97 + #def... 98 + })) 99 + // Top-level extra field not allowed (close provides non-recursive). 100 + t1: err: a & {f: int} @test(err, code=eval, path=f, contains="field not allowed", pos=[0:16]) 101 + // Nested extra field not allowed (__reclose detects recursive from #def). 102 + t2: err: a & {a: c: 1} @test(err, code=eval, path=a.c, contains="field not allowed", pos=[0:19]) 103 + // Allowed nested field works. 104 + t3: ok: a & {a: b: 5} @test(eq, {a: {b: 5}}) 105 + } 106 + 58 107 nonStructLiterals: { 59 108 // Test with non-struct should error 60 109 t1: err: __reclose("not a struct") @test(err, code=eval, contains="error in call to %s", args=[__reclose], pos=[0:11]) ··· 92 141 ./in.cue:36:4 93 142 [eval] nonRecursive.y.err.f: field not allowed: 94 143 ./in.cue:53:15 144 + [eval] nonRecursiveDetect.t1.err.f: field not allowed: 145 + ./in.cue:65:16 146 + [eval] mixedClose.t1.err.f: field not allowed: 147 + ./in.cue:85:16 148 + [eval] recloseWrapsClose.t1.err.f: field not allowed: 149 + ./in.cue:99:16 150 + [eval] recloseWrapsClose.t2.err.a.c: field not allowed: 151 + ./in.cue:101:19 95 152 [eval] nonStructLiterals.t1.err: error in call to __reclose: argument must be a struct or list literal: 96 - ./in.cue:59:11 153 + ./in.cue:108:11 97 154 [eval] nonStructLiterals.t4.err: error in call to __reclose: argument must be a struct or list literal: 98 - ./in.cue:63:11 155 + ./in.cue:112:11 99 156 [eval] nonStructDefinitions.t2.err: conflicting values {b:1} and 1 (mismatched types struct and int): 100 - ./in.cue:67:8 101 - ./in.cue:76:11 102 - ./in.cue:76:15 157 + ./in.cue:116:8 158 + ./in.cue:125:11 159 + ./in.cue:125:15 103 160 [eval] retro.t1.err: error in call to __reclose: __reclose may only be used when explicitopen is enabled: 104 161 ./old.cue:3:11 105 162 -- out/compile -- ··· 179 236 ok: (〈1;x〉 & { 180 237 d: { 181 238 f: int 239 + } 240 + }) 241 + } 242 + } 243 + nonRecursiveDetect: { 244 + foo: close({ 245 + b: {} 246 + }) 247 + a: __reclose({ 248 + 〈1;foo〉... 249 + }) 250 + t1: { 251 + err: (〈1;a〉 & { 252 + f: int 253 + }) 254 + } 255 + t2: { 256 + ok: (〈1;a〉 & { 257 + b: { 258 + c: 1 259 + } 260 + }) 261 + } 262 + } 263 + mixedClose: { 264 + #D: { 265 + x: { 266 + y: int 267 + } 268 + } 269 + foo: close({ 270 + b: {} 271 + }) 272 + a: __reclose({ 273 + 〈1;#D〉... 274 + 〈1;foo〉... 275 + }) 276 + t1: { 277 + err: (〈1;a〉 & { 278 + f: int 279 + }) 280 + } 281 + t2: { 282 + ok: (〈1;a〉 & { 283 + x: { 284 + y: 5 285 + } 286 + b: { 287 + c: 1 288 + } 289 + }) 290 + } 291 + } 292 + recloseWrapsClose: { 293 + #def: { 294 + a: { 295 + b: int 296 + } 297 + } 298 + a: __reclose(close({ 299 + 〈1;#def〉... 300 + })) 301 + t1: { 302 + err: (〈1;a〉 & { 303 + f: int 304 + }) 305 + } 306 + t2: { 307 + err: (〈1;a〉 & { 308 + a: { 309 + c: 1 310 + } 311 + }) 312 + } 313 + t3: { 314 + ok: (〈1;a〉 & { 315 + a: { 316 + b: 5 182 317 } 183 318 }) 184 319 }

+32 -9

internal/core/adt/closed.go

··· 83 83 return v.ClosedRecursive 84 84 } 85 85 86 - // ShouldRecursivelyClose reports whether this vertex should be closed 87 - // recursively using __reclose. This is to simulate compatibility mode 88 - // with the semantics from before explicitOpen was introduced. 89 - // 90 - // This is the case if any of the embeddings marked with ... were recursively 91 - // closed before opening them up with .... 92 - func (v *Vertex) ShouldRecursivelyClose() bool { 86 + // embedClosedness represents the strongest closedness level among vertices 87 + // embedded through a spread operator. Higher values take precedence. 88 + type embedClosedness uint8 89 + 90 + const ( 91 + embedOpen embedClosedness = iota 92 + embedNonRecursivelyClosed // from close() 93 + embedRecursivelyClosed // from definitions 94 + ) 95 + 96 + // setEmbedClosedness upgrades the embed closedness level based on the 97 + // closedness of arc. Higher levels take precedence; never downgrades. 98 + func (n *nodeContext) setEmbedClosedness(arc *Vertex) { 99 + switch { 100 + case arc.ClosedRecursive: 101 + n.embedClosedness = embedRecursivelyClosed 102 + case arc.ClosedNonRecursive && n.embedClosedness < embedNonRecursivelyClosed: 103 + n.embedClosedness = embedNonRecursivelyClosed 104 + } 105 + } 106 + 107 + // ApplyEmbedClosedness sets ClosedRecursive or ClosedNonRecursive on v 108 + // based on the strongest closedness level among vertices embedded through 109 + // a spread operator. This is used by __reclose. 110 + func (v *Vertex) ApplyEmbedClosedness() { 93 111 if v.state == nil { 94 - return false 112 + return 95 113 } 96 - return v.state.embedsRecursivelyClosed 114 + switch v.state.embedClosedness { 115 + case embedRecursivelyClosed: 116 + v.ClosedRecursive = true 117 + case embedNonRecursivelyClosed: 118 + v.ClosedNonRecursive = true 119 + } 97 120 } 98 121 99 122 // posInfo is a compact representation of position information for error reporting.

+5 -3

internal/core/adt/conjunct.go

··· 348 348 closeInfo.enclosingEmbed != 0 { 349 349 closeInfo.FromDef = false 350 350 } 351 - if arc.ClosedRecursive && c.CloseInfo.Opened { 352 - n.embedsRecursivelyClosed = true 351 + if c.CloseInfo.Opened { 352 + n.setEmbedClosedness(arc) 353 353 } 354 354 355 355 // disjunctions, we need to dereference he underlying node. ··· 374 374 switch isDef, _ := IsDef(c.Expr()); { 375 375 case isDef || arc.Label.IsDef() || closeInfo.TopDef: 376 376 if c.CloseInfo.Opened { 377 - n.embedsRecursivelyClosed = true 377 + // Definitions are always recursively closed, even if arc 378 + // doesn't have ClosedRecursive set yet at this point. 379 + n.embedClosedness = embedRecursivelyClosed 378 380 } 379 381 n.isDef = true 380 382 // n.node.ClosedRecursive = true // TODO: should we set this here?

+5 -6

internal/core/adt/eval.go

··· 341 341 // node after a corresponding task has been completed. 342 342 toComplete bool 343 343 344 - // embedsRecursivelyClosed is used to implement __reclose. It must be set 345 - // when a vertex that is recursively closed is embedded through a spread 346 - // operator. It is okay to set it if it is just unified with a vertex that 347 - // is recursively closed, but not added through a spread operator. The 348 - // result will just be an unnecessary call to __reclose. 349 - embedsRecursivelyClosed bool 344 + // embedClosedness tracks the strongest closedness level among vertices 345 + // embedded through a spread operator. It is used to determine whether 346 + // __reclose or __closeAll wrapping is needed. Higher values take 347 + // precedence; use setEmbedClosedness to update. 348 + embedClosedness embedClosedness 350 349 351 350 // isCompleting > 0 indicates whether a call to completeNodeTasks is in 352 351 // progress.

+12 -5

internal/core/compile/builtin.go

··· 188 188 c := call.OpContext() 189 189 190 190 x := call.Expr(0) 191 - switch x.(type) { 191 + switch x := x.(type) { 192 192 case *adt.StructLit, *adt.ListLit: 193 193 if src := x.Source(); src == nil || !src.Pos().Experiment().ExplicitOpen { 194 194 // Allow usage if explicit open is set 195 195 return c.NewErrf("__reclose may only be used when explicitopen is enabled") 196 196 } 197 + case *adt.CallExpr: 198 + // Allow __reclose(close(...)) so that __reclose can wrap 199 + // a close() call and conditionally add recursive closing. 200 + if b, ok := x.Fun.(*adt.Builtin); !ok || b.Name != "close" { 201 + return c.NewErrf("argument must be a struct or list literal, or a close() call") 202 + } 203 + if src := x.Source(); src == nil || !src.Pos().Experiment().ExplicitOpen { 204 + return c.NewErrf("__reclose may only be used when explicitopen is enabled") 205 + } 197 206 default: 198 207 return c.NewErrf("argument must be a struct or list literal") 199 208 } 200 - 201 - // must be literal struct 202 209 203 210 // Note that we could have an embedded scalar here, so having a struct 204 211 // or list does not guarantee that the result is that as well. ··· 207 214 // a: __reclose({ #Def }) 208 215 // 209 216 arg := call.Value(0) 210 - if s, ok := arg.(*adt.Vertex); ok && s.ShouldRecursivelyClose() { 211 - s.ClosedRecursive = true 217 + if s, ok := arg.(*adt.Vertex); ok { 218 + s.ApplyEmbedClosedness() 212 219 } 213 220 214 221 return arg

Configure Feed

Configure Feed