Skip to content

Commit

Permalink
internal/core/export: fix dereferencing bug
Browse files Browse the repository at this point in the history
In a previous fix we posponed the dereferencing of
a value, because we needed the non-dereferenced value
of the ArcType.

As it turns out, we also need the non-dereferenced value
for the docs and attributes. This change fixes that.

Note that the test has some modifications for a debug
testing. We leave this in for convenience down the line.
We leave the snippet itself to get an idea of the reducer
that was used to fix the code.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I59c97bfdb53918cd83b88be23a3a4bc5391fdb6f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194651
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
mpvl committed May 13, 2024
1 parent 645d586 commit efd38e6
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 203 deletions.
122 changes: 13 additions & 109 deletions internal/core/export/testdata/main/attrs.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ dynamicSimple: {
} @field(2) @field(1) @field(4) @field(3) @field(5)
doNotPropagate: {
#A: {} @attr1()
a: {} @attr1()
a: {}

// Do not accumulated field attributes in embedding.
#B: {} @attr1()
Expand Down Expand Up @@ -399,7 +399,7 @@ dynamicSimple: {
} @field(2) @field(1) @field(4) @field(3) @field(5)
doNotPropagate: {
#A: {} @attr1()
a: {} @attr1()
a: {}
#B: {} @attr1()
b: {}
}
Expand Down Expand Up @@ -456,35 +456,10 @@ dynamicSimple: {
diff old new
--- old
+++ new
@@ -125,44 +125,45 @@
} @field(2) @field(1) @field(4) @field(3) @field(5)
doNotPropagate: {
#A: {} @attr1()
- a: {}
-
- // Do not accumulated field attributes in embedding.
- #B: {} @attr1()
- b: {}
- }
- embedScalarField: {
- a: 2 @attr1() @attr2()
- }
- embedScalarDecl: {
- b0: {
- 2, @attr1()
- }
- b1: {
- 2, @attr1()
- }
- b2: {
- 2, @attr2(), @attr1()
- }
- }
- dontMergeForDef: {
- a: {
- @decl1()
- }
- b: {
@@ -150,19 +150,20 @@
@decl1()
}
b: {
- @decl1(), @decl2()
- x: 1
- }
Expand All @@ -498,31 +473,6 @@ diff old new
- comprehensions: {
- @step(0)
- @step(2c)
+ a: {} @attr1()
+
+ // Do not accumulated field attributes in embedding.
+ #B: {} @attr1()
+ b: {}
+ }
+ embedScalarField: {
+ a: 2 @attr1() @attr2()
+ }
+ embedScalarDecl: {
+ b0: {
+ 2, @attr1()
+ }
+ b1: {
+ 2, @attr1()
+ }
+ b2: {
+ 2, @attr2(), @attr1()
+ }
+ }
+ dontMergeForDef: {
+ a: {
+ @decl1()
+ }
+ b: {
+ @decl2(), @decl1()
+ x: 1
+ }
Expand All @@ -540,33 +490,10 @@ diff old new
c1: {} @step(1)
c2: {
@step(2a)
@@ -192,42 +193,43 @@
} @field(2) @field(1) @field(4) @field(3) @field(5)
doNotPropagate: {
#A: {} @attr1()
- a: {}
- #B: {} @attr1()
- b: {}
- }
- embedScalarField: {
- a: 2 @attr1() @attr2()
- }
- embedScalarDecl: {
- b0: {
- 2, @attr1()
- }
- b1: {
- 2, @attr1()
- }
- b2: {
- 2, @attr2(), @attr1()
- }
- }
- dontMergeForDef: {
- a: {
- @decl1()
- }
- b: {
@@ -215,19 +216,20 @@
@decl1()
}
b: {
- @decl1(), @decl2()
- x: 1
- }
Expand All @@ -580,29 +507,6 @@ diff old new
- comprehensions: {
- @step(0)
- @step(2c)
+ a: {} @attr1()
+ #B: {} @attr1()
+ b: {}
+ }
+ embedScalarField: {
+ a: 2 @attr1() @attr2()
+ }
+ embedScalarDecl: {
+ b0: {
+ 2, @attr1()
+ }
+ b1: {
+ 2, @attr1()
+ }
+ b2: {
+ 2, @attr2(), @attr1()
+ }
+ }
+ dontMergeForDef: {
+ a: {
+ @decl1()
+ }
+ b: {
+ @decl2(), @decl1()
+ x: 1
+ }
Expand All @@ -621,9 +525,9 @@ diff old new
c2: {
@step(2a)
-- diff/value/todo/p1 --
== Eval.doNotPropagate
.a: missing attribute.
#B: missing comment
comprehension: @step(4c) added in comprehensions
-- diff/value/todo/p3 --
Reordering of attributes.
-- out/value --
== Simplified
{
Expand Down
92 changes: 0 additions & 92 deletions internal/core/export/testdata/main/def.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -38,98 +38,6 @@ g: #d

[g e]
[g f]
-- out/value-v3 --
== Simplified
{
a: *2 | int
c: {}
g: {
e!: =~"a"
}
}
== Raw
{
a: *2 | int
b?: 4 | 5
c: {}
#d: {
e!: =~"a"
f?: 1
}
g: {
e!: =~"a"
f?: 1
}
}
== Final
{
a: 2
c: {}
g: {
e!: =~"a"
}
}
== All
{
a: *2 | int
b?: 4 | 5
c: {}
#d: {
e!: =~"a"
f?: 1
}
g: {
e!: =~"a"
f?: 1
}
}
== Eval
{
a: 2
b?: 4 | 5
c: {}
#d: {
e!: =~"a"
f?: 1
}
g: {
e!: =~"a"
f?: 1
}
}
-- diff/-out/value-v3<==>+out/value --
diff old new
--- old
+++ new
@@ -2,8 +2,6 @@
{
a: *2 | int
c: {}
-
- // Issue #2305
g: {
e!: =~"a"
}
@@ -17,8 +15,6 @@
e!: =~"a"
f?: 1
}
-
- // Issue #2305
g: {
e!: =~"a"
f?: 1
@@ -41,8 +37,6 @@
e!: =~"a"
f?: 1
}
-
- // Issue #2305
g: {
e!: =~"a"
f?: 1
-- diff/value/todo/p1 --
Missing comments.
-- out/value --
== Simplified
{
Expand Down
3 changes: 1 addition & 2 deletions internal/core/export/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,7 @@ func (e *exporter) structComposite(v *adt.Vertex, attrs []*ast.Attribute) ast.Ex

internal.SetConstraint(f, arc.ArcType.Token())

arc = arc.DerefValue()
f.Value = e.vertex(arc)
f.Value = e.vertex(arc.DerefValue())

if label.IsDef() {
e.inDefinition--
Expand Down
10 changes: 10 additions & 0 deletions internal/core/export/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,21 @@ var exclude = map[string]string{
}

func TestValue(t *testing.T) {
const debugValue = `
-- in.cue --
// Issue #2305
g: #d
#d: 1
`

test := cuetxtar.TxTarTest{
Root: "./testdata/main",
Name: "value",
Skip: exclude,
Matrix: cuetdtest.SmallMatrix,

// Uncomment to debug an isolated test case.
// DebugArchive: debugValue,
}

test.Run(t, func(t *cuetxtar.Test) {
Expand Down

0 comments on commit efd38e6

Please sign in to comment.