Skip to content

Commit

Permalink
internal/diff: clean up code a bit ahead of dropping deprecated APIs
Browse files Browse the repository at this point in the history
The diff package uses the deprecated cue.Value.Struct API
to deal with structs, and it's the only non-test piece of code we have
which still does this.

Ahead of doing that refactor, and to wrap my head around the code,
do a few small simplifications and tweaks first:

* xMap only needs to track the presence of fields by selector in x
* yMap using int makes the code below more consistent
* use tighter variable scopes where possible
* slightly reduce verbosity elsewhere where it makes sense

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I95603e6760ebf6ea777434c2dcfd79f769fe34d4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202084
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
  • Loading branch information
mvdan committed Oct 2, 2024
1 parent 5ffd573 commit 8009b56
Showing 1 changed file with 17 additions and 31 deletions.
48 changes: 17 additions & 31 deletions internal/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,33 +222,28 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) {
// We assume that the order of the elements of each value indicate an edge
// in the graph. This means that only the next unprocessed nodes can be
// those with no incoming edges.
xMap := make(map[string]int32, sx.Len())
yMap := make(map[string]int32, sy.Len())
xMap := make(map[string]struct{}, sx.Len())
yMap := make(map[string]int, sy.Len())
for i := 0; i < sx.Len(); i++ {
f, ok := d.field(sx, i)
if !ok {
continue
if ok {
xMap[f.Selector] = struct{}{}
}
xMap[f.Selector] = int32(i + 1)
}
for i := 0; i < sy.Len(); i++ {
f, ok := d.field(sy, i)
if !ok {
continue
if ok {
yMap[f.Selector] = i + 1
}
yMap[f.Selector] = int32(i + 1)
}

edits := []Edit{}
differs := false

var xi, yi int
var xf, yf cue.FieldInfo
var ok bool
for xi < sx.Len() || yi < sy.Len() {
for xi, yi := 0, 0; xi < sx.Len() || yi < sy.Len(); {
// Process zero nodes
for ; xi < sx.Len(); xi++ {
xf, ok = d.field(sx, xi)
xf, ok := d.field(sx, xi)
if !ok {
continue
}
Expand All @@ -260,16 +255,15 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) {
differs = true
}
for ; yi < sy.Len(); yi++ {
yf, ok = d.field(sy, yi)
yf, ok := d.field(sy, yi)
if !ok {
continue
}
if yMap[yf.Selector] == 0 {
// already done
continue
}
xp := xMap[yf.Selector]
if xp > 0 {
if _, ok := xMap[yf.Selector]; ok {
break
}
yMap[yf.Selector] = 0
Expand All @@ -278,43 +272,35 @@ func (d *differ) diffStruct(x, y cue.Value) (Kind, *EditScript) {
}

// Compare nodes
var ok bool
for ; xi < sx.Len(); xi++ {
xf, ok = d.field(sx, xi)
xf, ok := d.field(sx, xi)
if !ok {
continue
}

yp := yMap[xf.Selector]
if yp == 0 {
break
}
// If yp != xi+1, the topological sort was not possible.
yMap[xf.Selector] = 0

yf, ok := d.field(sy, int(yp-1))
yf, ok := d.field(sy, yp-1)
if !ok {
continue
}

kind := Identity
var kind Kind
var script *EditScript
switch {
case xf.IsDefinition != yf.IsDefinition,
xf.IsOptional != yf.IsOptional:
case xf.IsDefinition != yf.IsDefinition, xf.IsOptional != yf.IsOptional:
kind = Modified

default:
xv := xf.Value
yv := yf.Value
// TODO(perf): consider evaluating lazily.
kind, script = d.diffValue(xv, yv)
kind, script = d.diffValue(xf.Value, yf.Value)
}

edits = append(edits, Edit{kind, int32(xi + 1), yp, script})
if kind != Identity {
differs = true
}
edits = append(edits, Edit{kind, int32(xi + 1), int32(yp), script})
differs = differs || kind != Identity
}
}
if !differs {
Expand Down

0 comments on commit 8009b56

Please sign in to comment.