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
  • Loading branch information
mvdan committed Dec 27, 2024
1 parent 22bec0c commit 31b4380
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 31b4380

Please sign in to comment.