Skip to content

Commit

Permalink
cue/token: add Pos.Compare from toposort
Browse files Browse the repository at this point in the history
Now that Go is moving towards int-based compare functions,
such as the general cmp.Compare or specific methods like
net/netip.AddrPort.Compare, add one of our own as well.

Sorting by position is rather common, particularly when wanting to
sort nodes, errors, or values by position. One such example is
toposort, where we borrow the initial code from, as it seems
like the most principled out of them.

Follow-up commits will switch other position compare uses to
cue/token.Pos.Compare, split up for the sake of being able to
more easily bisect or investigate any potential issues.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Iaa8483c11cb1ef784c704630631a167140b25efe
Dispatch-Trailer: {"type":"trybot","CL":1206368,"patchset":2,"ref":"refs/changes/68/1206368/2","targetBranch":"master"}
  • Loading branch information
mvdan authored and cueckoo committed Dec 27, 2024
1 parent 22bec0c commit 7b8a344
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 16 deletions.
25 changes: 25 additions & 0 deletions cue/token/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package token

import (
"cmp"
"fmt"
"sort"
"sync"
Expand Down Expand Up @@ -74,6 +75,10 @@ func (p Pos) File() *File {
return p.file
}

// TODO(mvdan): The methods below don't need to build an entire Position
// just to access some of the information. This could matter particularly for
// Compare, as it is called many times when sorting by position.

func (p Pos) Line() int {
if p.file == nil {
return 0
Expand Down Expand Up @@ -106,6 +111,26 @@ func (p Pos) String() string {
return p.Position().String()
}

// Compare returns an integer comparing two positions. The result will be 0 if p == p2,
// -1 if p < p2, and +1 if p > p2. Note that [NoPos] is always smaller than any valid position.
func (p Pos) Compare(p2 Pos) int {
if p == p2 {
return 0
} else if p == NoPos {
return 1
} else if p2 == NoPos {
return -1
}
pos, pos2 := p.Position(), p2.Position()
if c := cmp.Compare(pos.Filename, pos2.Filename); c != 0 {
return c
}
// Note that CUE doesn't currently use any directives which alter
// position information, like Go's //line, so comparing by offset is enough.
return cmp.Compare(pos.Offset, pos2.Offset)

}

// NoPos is the zero value for Pos; there is no file and line information
// associated with it, and [Pos.IsValid] is false. NoPos is always
// smaller than any other Pos value. The corresponding Position value
Expand Down
17 changes: 1 addition & 16 deletions internal/core/toposort/vertex.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ package toposort
// Vertex's slice of StructInfos.

import (
"cmp"
"fmt"
"slices"

Expand Down Expand Up @@ -368,7 +367,7 @@ type vertexFeatures struct {
}

func (vf *vertexFeatures) compareStructMeta(a, b *structMeta) int {
if c := comparePos(a.pos, b.pos); c != 0 {
if c := a.pos.Compare(b.pos); c != 0 {
return c
}
aHasDyn := a.hasDynamic(vf.dynFieldsMap)
Expand All @@ -383,20 +382,6 @@ func (vf *vertexFeatures) compareStructMeta(a, b *structMeta) int {
}
}

func comparePos(aPos, bPos token.Pos) int {
if aPos == bPos {
return 0
} else if aPos == token.NoPos {
return 1
} else if bPos == token.NoPos {
return -1
}
if c := cmp.Compare(aPos.Filename(), bPos.Filename()); c != 0 {
return c
}
return cmp.Compare(aPos.Offset(), bPos.Offset())
}

func VertexFeatures(ctx *adt.OpContext, v *adt.Vertex) []adt.Feature {
debug("\n*** V (%s %v %p) ***\n", v.Label.RawString(ctx), v.Label, v)

Expand Down

0 comments on commit 7b8a344

Please sign in to comment.