Skip to content

Commit

Permalink
pgraph, lang: ast: Fix failing tests due to non-deterministic topo sort
Browse files Browse the repository at this point in the history
This causes inconsistent type unification when running our tests. It's a
bad user experience too.
  • Loading branch information
purpleidea committed Jul 1, 2024
1 parent 2b8d08e commit 7faab88
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 5 deletions.
6 changes: 4 additions & 2 deletions lang/ast/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3829,8 +3829,10 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error {
orderingGraphSingleton = false
}

//nodeOrder, err := orderingGraphFiltered.TopologicalSort()
nodeOrder, err := orderingGraph.TopologicalSort()
// If we don't do this deterministically the type unification errors can
// flip from `type error: Int != Str` to `type error: Str != Int` etc...
nodeOrder, err := orderingGraph.DeterministicTopologicalSort() // sorted!

if err != nil {
// TODO: print the cycle in a prettier way (with names?)
if obj.data.Debug {
Expand Down
2 changes: 1 addition & 1 deletion lang/interpret_test/TestAstFunc2/polymorphic-lambda.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ $out2 = $add($val) # hellohello

test [fmt.printf("%s + %s is %s", $val, $val, $out2),] {} # simple concat
-- OUTPUT --
# err: errUnify: unify error with: topLevel(singleton(func(x) { call:_operator(str("+"), var(x), var(x)) })): type error: Str != Int
# err: errUnify: unify error with: topLevel(singleton(func(x) { call:_operator(str("+"), var(x), var(x)) })): type error: Int != Str
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ test "test2" {
anotherstr => $id("hello"),
}
-- OUTPUT --
# err: errUnify: unify error with: topLevel(singleton(func(x) { var(x) })): type error: Str != Int
# err: errUnify: unify error with: topLevel(singleton(func(x) { var(x) })): type error: Int != Str
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ class use_polymorphically($id) {
}
include use_polymorphically(func($x) {$x})
-- OUTPUT --
# err: errUnify: unify error with: topLevel(singleton(func(x) { var(x) })): type error: Str != Int
# err: errUnify: unify error with: topLevel(singleton(func(x) { var(x) })): type error: Int != Str
59 changes: 59 additions & 0 deletions pgraph/pgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,65 @@ func (g *Graph) TopologicalSort() ([]Vertex, error) { // kahn's algorithm
return L, nil
}

// DeterministicTopologicalSort returns the sort of graph vertices in a stable
// topological sort order. It's slower than the TopologicalSort implementation,
// but guarantees that two identical graphs produce the same sort each time.
// TODO: add memoization, and cache invalidation to speed this up :)
func (g *Graph) DeterministicTopologicalSort() ([]Vertex, error) { // kahn's algorithm
var L []Vertex // empty list that will contain the sorted elements
var S []Vertex // set of all nodes with no incoming edges
remaining := make(map[Vertex]int) // amount of edges remaining

var vertices []Vertex
indegree := g.InDegree()
for k := range indegree {
vertices = append(vertices, k)
}
sort.Sort(VertexSlice(vertices)) // add determinism
//for v, d := range g.InDegree()
for _, v := range vertices { // map[Vertex]int
d := indegree[v]
if d == 0 {
// accumulate set of all nodes with no incoming edges
S = append(S, v)
} else {
// initialize remaining edge count from indegree
remaining[v] = d
}
}

for len(S) > 0 {
last := len(S) - 1 // remove a node v from S
v := S[last]
S = S[:last]
L = append(L, v) // add v to tail of L
// This doesn't need to loop in a deterministically sorted order.
for n := range g.adjacency[v] { // map[Vertex]Edge
// for each node n remaining in the graph, consume from
// remaining, so for remaining[n] > 0
if remaining[n] > 0 {
remaining[n]-- // remove edge from the graph
if remaining[n] == 0 { // if n has no other incoming edges
S = append(S, n) // insert n into S
}
}
}
}

// if graph has edges, eg if any value in rem is > 0
for c, in := range remaining {
if in > 0 {
for n := range g.adjacency[c] {
if remaining[n] > 0 {
return nil, ErrNotAcyclic
}
}
}
}

return L, nil
}

// Reachability finds the shortest path in a DAG from a to b, and returns the
// slice of vertices that matched this particular path including both a and b.
// It returns nil if a or b is nil, and returns empty list if no path is found.
Expand Down

0 comments on commit 7faab88

Please sign in to comment.