Skip to content

Commit e96867d

Browse files
authored
Merge pull request #3142 from ruby/method-builder-dfs
Replace the method sorter with a direct DFS
2 parents 52b9107 + 56699e8 commit e96867d

2 files changed

Lines changed: 34 additions & 30 deletions

File tree

lib/rbs/definition_builder/method_builder.rb

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,40 +46,47 @@ def validate!
4646
self
4747
end
4848

49-
def each
50-
if block_given?
51-
Sorter.new(methods).each_strongly_connected_component do |scc|
52-
if scc.size > 1
53-
raise RecursiveAliasDefinitionError.new(type: type, defs: scc)
49+
def each(&block)
50+
if block
51+
# Yields the original method of an alias before the alias, like the
52+
# topological sort did, and detects recursive alias definitions on the way
53+
if methods.each_value.any? {|defn| defn.original.is_a?(AST::Members::Alias) }
54+
done = {} #: Hash[Definition, bool]
55+
done.compare_by_identity
56+
methods.each_value do |defn|
57+
each_alias_first(defn, done, [], &block)
5458
end
55-
56-
yield scc[0]
59+
else
60+
methods.each_value(&block)
5761
end
5862
else
5963
enum_for :each
6064
end
6165
end
6266

63-
class Sorter
64-
include TSort
67+
private
6568

66-
attr_reader :methods
69+
def each_alias_first(defn, done, visiting, &block)
70+
return if done[defn]
6771

68-
def initialize(methods)
69-
@methods = methods
72+
if visiting.any? {|other| other.equal?(defn) }
73+
index = visiting.index {|other| other.equal?(defn) } or raise
74+
raise RecursiveAliasDefinitionError.new(type: type, defs: visiting[index..] || raise)
7075
end
7176

72-
def tsort_each_node(&block)
73-
methods.each_value(&block)
74-
end
75-
76-
def tsort_each_child(defn)
77-
if (member = defn.original).is_a?(AST::Members::Alias)
78-
if old = methods[member.old_name]
79-
yield old
77+
if (member = defn.original).is_a?(AST::Members::Alias)
78+
if old = methods.fetch(member.old_name, nil)
79+
# A self alias forms a size-1 SCC that the topological sort yielded as is
80+
unless old.equal?(defn)
81+
visiting.push(defn)
82+
each_alias_first(old, done, visiting, &block)
83+
visiting.pop
8084
end
8185
end
8286
end
87+
88+
done[defn] = true
89+
yield defn
8390
end
8491
end
8592

sig/method_builder.rbs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,14 @@ module RBS
4747
def each: () { (Definition) -> void } -> void
4848
| () -> Enumerator[Definition, void]
4949

50-
class Sorter
51-
include TSort[Definition]
50+
private
5251

53-
attr_reader methods: Hash[Symbol, Definition]
54-
55-
def initialize: (Hash[Symbol, Definition]) -> void
56-
57-
def tsort_each_node: { (Definition) -> void } -> void
58-
59-
def tsort_each_child: (Definition) { (Definition) -> void } -> void
60-
end
52+
# Yields the definition, recursively yielding the original of an alias first
53+
#
54+
# Raises `RecursiveAliasDefinitionError` when the aliases form a cycle of two or
55+
# more methods.
56+
#
57+
def each_alias_first: (Definition, Hash[Definition, bool] done, Array[Definition] visiting) { (Definition) -> void } -> void
6158
end
6259

6360
attr_reader env: Environment

0 commit comments

Comments
 (0)