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 2, 2023
1 parent 6717307 commit d5b7c98
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
21 changes: 15 additions & 6 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,21 +151,30 @@ 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

# Capture this Hash locally, to mutate it in the `inherited` callback below.
generic_instances = @generic_instances

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)
owner.send(:define_method, :inherited) do |new_subclass|
# 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
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 d5b7c98

Please sign in to comment.