Skip to content

Commit a09a38b

Browse files
committed
internal/cuedebug: drop openinline flag
The two Unity projects which prompted us to originally introduce the openinline flag for backwards compatibility with evalv2 were github.com/tmm1/taxes.cue and Judson's private project. As of master today, the former passes entirely, and the latter passes with the work-in-progress change at https://cuelang.org/cl/1212514. Given the above, the openinline flag should no longer be used by anyone, and leaving it around may just cause confusion down the line for both CUE users and maintainers. As further proof that the option is no longer needed, both issue3688.txtar and openinline.txtar are entirely unchanged. See #3688. Signed-off-by: Daniel Martí <[email protected]> Change-Id: Ia6945ba7dd992d8467fa25f61ce01b8af030c270 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1213042 Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Marcel van Lohuizen <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
1 parent f836184 commit a09a38b

File tree

7 files changed

+7
-65
lines changed

7 files changed

+7
-65
lines changed

cue/testdata/eval/issue3688.txtar

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
# This test is important in locking in the behaviour of the now-defalut
2-
# CUE_DEBUG=openinline=0.
1+
# This test is important in locking in the behaviour of evalv3,
2+
# which fixed some closedness bugs present in the older evalv2.
33

44
-- x.cue --
55
package x

cue/testdata/eval/openinline.txtar

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
#openInline: true
1+
# This test is important in locking in the closedness behavior
2+
# of evalv2, where inline structs cause some closedness errors
3+
# to be ignored. evalv3 mimics this behavior for the sake of
4+
# backwards compatibility and not breaking existing users.
25

36
-- cue.mod/module.cue --
47
module: "mod.test"

internal/core/adt/conjunct.go

-12
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,6 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
4141
if c.CloseInfo.FromDef {
4242
n.node.ClosedRecursive = true
4343
}
44-
// NOTE: the check for OpenInline is not strictly necessary, but it
45-
// clarifies that using id.FromEmbed is not used when OpenInline is not
46-
// used.
47-
// TODO(dynclose): we have disabled OpenInline for the new dynamic
48-
// closedness to be more inline with V2 and because it works slightly
49-
// differently. Consider reinstating this in some shape or form.
50-
// if c.CloseInfo.FromEmbed || (n.ctx.OpenInline && id.FromEmbed) {
51-
// if !n.ctx.OpenInline {
52-
// c.CloseInfo.FromEmbed = false
53-
// id.FromEmbed = false
54-
// }
55-
// }
5644

5745
// TODO: consider setting this as a safety measure.
5846
// if c.CloseInfo.CycleType > id.CycleType {

internal/core/adt/context.go

-13
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,6 @@ type OpContext struct {
162162
// enabled.
163163
inConstraint int
164164

165-
// inLiteralSelectee indicates that we are evaluating a literal struct
166-
// as the receiver of a selector. This is used to turn off closedness
167-
// checking in compatibility mode.
168-
inLiteralSelectee int
169-
170165
// inDetached indicates that inline structs evaluated in the current context
171166
// should never be shared. This is the case, for instance, with the source
172167
// for the for clause in a comprehension.
@@ -1070,14 +1065,6 @@ func (c *OpContext) node(orig Node, x Expr, scalar bool, state combinedFlags) *V
10701065
defer func() { c.ci.FromDef = saved }()
10711066
}
10721067

1073-
if c.OpenInline {
1074-
if _, ok := x.(Resolver); !ok {
1075-
c.ci.FromEmbed = true
1076-
c.inLiteralSelectee++
1077-
defer func() { c.inLiteralSelectee-- }()
1078-
}
1079-
}
1080-
10811068
// TODO: always get the vertex. This allows a whole bunch of trickery
10821069
// down the line.
10831070
v := c.unifyNode(x, state)

internal/core/adt/eval_test.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,6 @@ func skipFiles(a ...*ast.File) (reason string) {
139139
}
140140

141141
func runEvalTest(t *cuetxtar.Test, version internal.EvaluatorVersion, flags cuedebug.Config) (errorCount int) {
142-
// If #openInline is set, override the default as set by the cuedebug package.
143-
if s, ok := t.Value("openInline"); ok {
144-
flags.OpenInline = s == "true"
145-
}
146-
147142
a := t.Instance()
148143
r := runtime.NewWithSettings(version, flags)
149144

@@ -220,8 +215,7 @@ func TestX(t *testing.T) {
220215

221216
flags := cuedebug.Config{
222217
Sharing: true, // Uncomment to turn sharing off.
223-
// OpenInline: true,
224-
LogEval: 1, // Uncomment to turn logging off
218+
LogEval: 1, // Uncomment to turn logging off
225219
}
226220

227221
version := internal.DefaultVersion

internal/core/adt/fields.go

-6
Original file line numberDiff line numberDiff line change
@@ -370,12 +370,6 @@ func (ctx *OpContext) addPositions(c Conjunct) {
370370
// notAllowedError reports a field not allowed error in n and sets the value
371371
// for arc f to that error.
372372
func (ctx *OpContext) notAllowedError(arc *Vertex) *Bottom {
373-
// TODO(compat): ultimately we should strive to remove this explicit
374-
// reproduction of a bug to ensure compatibility with the old evaluator.
375-
if ctx.inLiteralSelectee > 0 {
376-
return nil
377-
}
378-
379373
defer ctx.PopArc(ctx.PushArc(arc))
380374

381375
defer ctx.ReleasePositions(ctx.MarkPositions())

internal/cuedebug/cuedebug.go

-24
Original file line numberDiff line numberDiff line change
@@ -42,30 +42,6 @@ type Config struct {
4242
// lexicographically.
4343
SortFields bool
4444

45-
// OpenInline permits disallowed fields to be selected into literal structs
46-
// that would normally result in a close error. For instance,
47-
//
48-
// #D: {a: 1}
49-
// x: (#D & {b: 2}).b // allow this
50-
//
51-
// This behavior was erroneously permitted in the v2 evaluator and was fixed
52-
// in v3. This allows users that rely on this behavior to use v3. This
53-
// option also discards closedness of the resulting expression. As was
54-
// reported in Issue #3534, this was another erroneous behavior in v2 that
55-
// is otherwise fixed in v3.
56-
//
57-
// To aid the transition to v3, this is enabled by default for now.
58-
//
59-
// A possible solution for both incompatibilities would be the introduction
60-
// of an openAll builtin to recursive open up a cue value. For the first
61-
// issue, the example above could be rewritten as:
62-
//
63-
// x: (openAll(#D) & {b: 2}).b
64-
//
65-
// For the second issue, to open up the entire result of an inline struct,
66-
// such an expression could be written as `openAll(expr).out`.
67-
OpenInline bool
68-
6945
// OpenDef disables the check for closedness of definitions.
7046
OpenDef bool
7147
}

0 commit comments

Comments
 (0)