From 7faab882e552ea0a77be736b53cca686f272c94b Mon Sep 17 00:00:00 2001 From: James Shubin Date: Wed, 26 Jun 2024 21:25:48 -0400 Subject: [PATCH] pgraph, lang: ast: Fix failing tests due to non-deterministic topo sort This causes inconsistent type unification when running our tests. It's a bad user experience too. --- lang/ast/structs.go | 6 +- .../TestAstFunc2/polymorphic-lambda.txtar | 2 +- .../TestAstFunc2/test-monomorphic-bind.txtar | 2 +- .../test-monomorphic-class-arg.txtar | 2 +- pgraph/pgraph.go | 59 +++++++++++++++++++ 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/lang/ast/structs.go b/lang/ast/structs.go index d4cf81258..bea42a3c0 100644 --- a/lang/ast/structs.go +++ b/lang/ast/structs.go @@ -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 { diff --git a/lang/interpret_test/TestAstFunc2/polymorphic-lambda.txtar b/lang/interpret_test/TestAstFunc2/polymorphic-lambda.txtar index 318f7f20e..482a05e01 100644 --- a/lang/interpret_test/TestAstFunc2/polymorphic-lambda.txtar +++ b/lang/interpret_test/TestAstFunc2/polymorphic-lambda.txtar @@ -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 diff --git a/lang/interpret_test/TestAstFunc2/test-monomorphic-bind.txtar b/lang/interpret_test/TestAstFunc2/test-monomorphic-bind.txtar index 67626649d..a1751059e 100644 --- a/lang/interpret_test/TestAstFunc2/test-monomorphic-bind.txtar +++ b/lang/interpret_test/TestAstFunc2/test-monomorphic-bind.txtar @@ -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 diff --git a/lang/interpret_test/TestAstFunc2/test-monomorphic-class-arg.txtar b/lang/interpret_test/TestAstFunc2/test-monomorphic-class-arg.txtar index eaecfe923..63c4891af 100644 --- a/lang/interpret_test/TestAstFunc2/test-monomorphic-class-arg.txtar +++ b/lang/interpret_test/TestAstFunc2/test-monomorphic-class-arg.txtar @@ -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 diff --git a/pgraph/pgraph.go b/pgraph/pgraph.go index d31907ae0..d5689c0f7 100644 --- a/pgraph/pgraph.go +++ b/pgraph/pgraph.go @@ -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.