fix is_cyclic and topological_sort

src/traversals/dfs.jl

@@ -32,30 +32,30 @@ function is_cyclic end
 end
 # see https://github.com/mauro3/SimpleTraits.jl/issues/47#issuecomment-327880153 for syntax
 @traitfn function is_cyclic(g::AG::IsDirected) where {T,AG<:AbstractGraph{T}}
- # 0 if not visited, 1 if visited, 2 if in the current dfs path, 3 if fully explored 
+ # 0 if not visited, 1 if in the current dfs path, 2 if fully explored
  vcolor = zeros(UInt8, nv(g))
  vertex_stack = Vector{T}()
  for v in vertices(g)
  vcolor[v] != 0 && continue
  push!(vertex_stack, v)
- vcolor[v] = 1
  while !isempty(vertex_stack)
  u = vertex_stack[end]
- if vcolor[u] == 1
- vcolor[u] = 2
+ if vcolor[u] == 0
+ vcolor[u] = 1
  for n in outneighbors(g, u)
  # we hit a loop when reaching back a vertex of the main path
- if vcolor[n] == 2
+ if vcolor[n] == 1
  return true
  elseif vcolor[n] == 0
  # we store neighbors, but these are not yet on the path
- vcolor[n] = 1
  push!(vertex_stack, n)
  end
  end
  else
- vcolor[u] = 3
  pop!(vertex_stack)
+ if vcolor[u] == 1 
+ vcolor[u] = 2
+ end
  end
  end
  end
@@ -87,32 +87,33 @@ graph `g` as a vector of vertices in topological order.
 function topological_sort_by_dfs end
 # see https://github.com/mauro3/SimpleTraits.jl/issues/47#issuecomment-327880153 for syntax
 @traitfn function topological_sort_by_dfs(g::AG::IsDirected) where {T,AG<:AbstractGraph{T}}
- # 0 if not visited, 1 if visited, 2 if in the current dfs path, 3 if fully explored 
+ # 0 if not visited, 1 if in the current dfs path, 2 if fully explored
  vcolor = zeros(UInt8, nv(g))
  verts = Vector{T}()
  vertex_stack = Vector{T}()
  for v in vertices(g)
  vcolor[v] != 0 && continue
  push!(vertex_stack, v)
- vcolor[v] = 1
  while !isempty(vertex_stack)
  u = vertex_stack[end]
- if vcolor[u] == 1
- vcolor[u] = 2
+ if vcolor[u] == 0
+ vcolor[u] = 1
  for n in outneighbors(g, u)
  # we hit a loop when reaching back a vertex of the main path
- if vcolor[n] == 2
+ if vcolor[n] == 1
  error("The input graph contains at least one loop.") # TODO 0.7 should we use a different error?
  elseif vcolor[n] == 0
  # we store neighbors, but these are not yet on the path
- vcolor[n] = 1
  push!(vertex_stack, n)
  end
  end
- else
- vcolor[u] = 3
+ else
  pop!(vertex_stack)
- pushfirst!(verts, u)
+ # if vcolor[u] == 2, the vertex was already explored and added to verts
+ if vcolor[u] == 1 
+ vcolor[u] = 2
+ pushfirst!(verts, u)
+ end
  end
  end
  end

test/traversals/dfs.jl

@@ -23,6 +23,15 @@
  0 0 0 1 0 0 0
  ],
  )
+ # non regression test following https://github.com/JuliaGraphs/Graphs.jl/pull/266#issuecomment-1621698039
+ g6 = SimpleDiGraph(4)
+ add_edge!(g6, 1, 2)
+ add_edge!(g6, 2, 3)
+ add_edge!(g6, 2, 4)
+ add_edge!(g6, 4, 3)
+ g7 = copy(g6)
+ add_edge!(g7, 3, 4)
+
  @testset "dfs_tree" begin
  for g in testdigraphs(g5)
  z = @inferred(dfs_tree(GenericDiGraph(g), 1))
@@ -45,6 +54,11 @@
  @test @inferred(is_cyclic(g))
  @test_throws ErrorException topological_sort(g)
  end
+
+ # non regression test following https://github.com/JuliaGraphs/Graphs.jl/pull/266#issuecomment-1621698039
+ for g in testdigraphs(g6)
+ @test @inferred(topological_sort(g)) == [1, 2, 4, 3]
+ end
  end
 
  @testset "topological_sort_by_dfs" begin
@@ -84,5 +98,9 @@
  for g in testgraphs(gloop_undirected)
  @test @inferred(is_cyclic(g))
  end
+ # non regression test following https://github.com/JuliaGraphs/Graphs.jl/pull/266#issuecomment-1621698039
+ for g in testdigraphs(g7)
+ @test @inferred(is_cyclic(g))
+ end
  end
 end