Skip to content

Commit

Permalink
Fix .inhertited infinite recursion bug
Browse files Browse the repository at this point in the history
  • Loading branch information
amomchilov committed Sep 1, 2023
1 parent fe4bca1 commit 8b9189b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 23 deletions.
38 changes: 21 additions & 17 deletions lib/tapioca/runtime/generic_type_registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def create_generic_type(constant, name)
# the generic class `Foo[Bar]` is still a `Foo`. That is:
# `Foo[Bar].new.is_a?(Foo)` should be true, which isn't the case
# if we just clone the class. But subclassing works just fine.
create_safe_subclass(constant)
create_safe_subclass(constant, name)
else
# This can only be a module and it is fine to just clone modules
# since they can't have instances and will not have `is_a?` relationships.
Expand Down Expand Up @@ -151,31 +151,35 @@ def create_generic_type(constant, name)
generic_type
end

sig { params(constant: T::Class[T.anything]).returns(T::Class[T.anything]) }
def create_safe_subclass(constant)
sig { params(constant: T::Class[T.anything], name: String).returns(T::Class[T.anything]) }
def create_safe_subclass(constant, name)
# Lookup the "inherited" class method
inherited_method = constant.method(:inherited)
# and the module that defines it
owner = inherited_method.owner

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

begin
# Otherwise, some inherited method could be preventing us
# from creating subclasses, so let's override it and rescue
owner.send(:define_method, :inherited) do |s|
inherited_method.call(s)
rescue
# Ignoring errors
end

# return a subclass
Class.new(constant)
ensure
# Reinstate the original inherited method back.
# Otherwise, some inherited method could be preventing us
# from creating subclasses, so let's override it and rescue
owner.send(:define_method, :inherited) do |new_subclass|
# Reinstate the original inherited method back ASAP
owner.send(:define_method, :inherited, inherited_method)

# Register this new subclass ASAP, to prevent re-entry into the `create_safe_subclass` code-path.
# This can happen if the sig of the original `.inherited` method references the generic type itself.
@generic_instances[name] ||= new_subclass

# Call the original `.inherited` method, but rescue any errors that might be raised,
# which would have otherwise prevented our subclass from being created.
inherited_method.call(new_subclass)
rescue
# Ignoring errors
end

# return a subclass
Class.new(constant)
end

sig { params(constant: Module).returns(T::Array[TypeVariableModule]) }
Expand Down
8 changes: 2 additions & 6 deletions spec/tapioca/runtime/generic_type_registry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,10 @@ class GenericTypeRegistrySpec < Minitest::Spec
_ = HasNonRecursiveInheritedSig[Object]
end

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

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

0 comments on commit 8b9189b

Please sign in to comment.