Skip to content

Commit 8b9189b

Browse files
committed
Fix .inhertited infinite recursion bug
1 parent fe4bca1 commit 8b9189b

File tree

2 files changed

+23
-23
lines changed

2 files changed

+23
-23
lines changed

lib/tapioca/runtime/generic_type_registry.rb

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def create_generic_type(constant, name)
117117
# the generic class `Foo[Bar]` is still a `Foo`. That is:
118118
# `Foo[Bar].new.is_a?(Foo)` should be true, which isn't the case
119119
# if we just clone the class. But subclassing works just fine.
120-
create_safe_subclass(constant)
120+
create_safe_subclass(constant, name)
121121
else
122122
# This can only be a module and it is fine to just clone modules
123123
# since they can't have instances and will not have `is_a?` relationships.
@@ -151,31 +151,35 @@ def create_generic_type(constant, name)
151151
generic_type
152152
end
153153

154-
sig { params(constant: T::Class[T.anything]).returns(T::Class[T.anything]) }
155-
def create_safe_subclass(constant)
154+
sig { params(constant: T::Class[T.anything], name: String).returns(T::Class[T.anything]) }
155+
def create_safe_subclass(constant, name)
156156
# Lookup the "inherited" class method
157157
inherited_method = constant.method(:inherited)
158158
# and the module that defines it
159159
owner = inherited_method.owner
160160

161-
# If no one has overriden the inherited method yet, just subclass
161+
# If no one has overridden the inherited method yet, just subclass
162162
return Class.new(constant) if Class == owner
163163

164-
begin
165-
# Otherwise, some inherited method could be preventing us
166-
# from creating subclasses, so let's override it and rescue
167-
owner.send(:define_method, :inherited) do |s|
168-
inherited_method.call(s)
169-
rescue
170-
# Ignoring errors
171-
end
172-
173-
# return a subclass
174-
Class.new(constant)
175-
ensure
176-
# Reinstate the original inherited method back.
164+
# Otherwise, some inherited method could be preventing us
165+
# from creating subclasses, so let's override it and rescue
166+
owner.send(:define_method, :inherited) do |new_subclass|
167+
# Reinstate the original inherited method back ASAP
177168
owner.send(:define_method, :inherited, inherited_method)
169+
170+
# Register this new subclass ASAP, to prevent re-entry into the `create_safe_subclass` code-path.
171+
# This can happen if the sig of the original `.inherited` method references the generic type itself.
172+
@generic_instances[name] ||= new_subclass
173+
174+
# Call the original `.inherited` method, but rescue any errors that might be raised,
175+
# which would have otherwise prevented our subclass from being created.
176+
inherited_method.call(new_subclass)
177+
rescue
178+
# Ignoring errors
178179
end
180+
181+
# return a subclass
182+
Class.new(constant)
179183
end
180184

181185
sig { params(constant: Module).returns(T::Array[TypeVariableModule]) }

spec/tapioca/runtime/generic_type_registry_spec.rb

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,10 @@ class GenericTypeRegistrySpec < Minitest::Spec
8383
_ = HasNonRecursiveInheritedSig[Object]
8484
end
8585

86-
it "FIXME: breaks from infinite recursion if the sig on .inherited uses the generic type" do
86+
it "works for classes whose .inherited sig reference the generic type itself" do
8787
# Our swizzled implementation of the `.inherited` method needs to be carefully implemented to not fall into
8888
# infinite recursion when the sig for the method references the class that it's defined on.
89-
assert_raises(SystemStackError) do
90-
HasRecursiveInheritedSig[Object]
91-
end
89+
_ = HasRecursiveInheritedSig[Object]
9290
end
9391
end
9492
end
@@ -120,8 +118,6 @@ class HasNonRecursiveInheritedSig
120118
class << self
121119
extend T::Sig
122120

123-
# The correct type would be `T::Class[SampleGenericClass[T.anything]]`, but that would crash Tapioca.
124-
# That's not honey Pooh, that's recursion!
125121
sig { params(subclass: T::Class[T.anything]).void }
126122
def inherited(subclass)
127123
super

0 commit comments

Comments
 (0)