Skip to content

Commit

Permalink
Stop generating more methods than necessary
Browse files Browse the repository at this point in the history
We used to treat methods that didn't have a source location the same as
methods that explicitly were not defined in the current gem. That
resulted in Tapioca creating more method definitions than necessary.

We would only skip method generation for a method if the constant it was
on was an ignored type (i.e. a built-in type), so that we wouldn't keep
redefining methods for built-in types. However, for all other types,
especially types that come from other gems, we would just keep on
generating all the methods regardless of if they were defined by this
gem or not.

Moreover, the source location check was happening at the wrong location,
before unwrapping the method signature. Thus, many methods with
signatures would not be generated when the previous problem was fixed,
since our code would see them as being defined in Sorbet runtime.

The fix is to return a more fine-grained result from `method_in_gem?`
which signals yes/no/don't-have-source-location. Based on that we can
skip generating don't-have-source-location cases if they are for
built-in types and totally ignore the methods that have a source
location and are definitely not defined in the current gem.

Additionally, if we try to unwrap the method signature and we get an
exception, that means the signature block raised an error. If we
continue with the method as is, the source location checks would think
the method definition does not belong in the gem (since the method is
still wrapped), and we would thus skip the method generation. To avoid
that, the `signature_for` method is not raising a custom exception to
signal that exceptional case, so that we can at least continue
generating "a" method definition.
  • Loading branch information
paracycle committed Apr 23, 2021
1 parent 6e4f769 commit 593bfa7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 22 deletions.
33 changes: 25 additions & 8 deletions lib/tapioca/compilers/symbol_table/symbol_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,25 @@ def struct_method?(constant, method_name)
def compile_method(symbol_name, constant, method)
return unless method
return unless method.owner == constant
return if symbol_ignored?(symbol_name) && !method_in_gem?(method)

signature = signature_of(method)
method = T.let(signature.method, UnboundMethod) if signature
begin
signature = signature_of(method)
method = T.let(signature.method, UnboundMethod) if signature

case method_in_gem?(method)
when nil
# This means that this is a C-method. Thus, we want to
# skip it only if the constant is an ignored one, since
# that probably means that we've hit a C-method for a
# core type.
return if symbol_ignored?(symbol_name)
when false
# Do not process this method, if it is not defined by the current gem
return
end
rescue SignatureBlockError
signature = nil
end

method_name = method.name.to_s
return unless valid_method_name?(method_name)
Expand Down Expand Up @@ -747,12 +762,12 @@ def indented(str)
" " * @indent + str
end

sig { params(method: UnboundMethod).returns(T::Boolean) }
sig { params(method: UnboundMethod).returns(T.nilable(T::Boolean)) }
def method_in_gem?(method)
source_location = method.source_location&.first
return false if source_location.nil?
source_location, _ = method.source_location
return nil unless source_location

gem.contains_path?(source_location)
source_location == "(eval)" || gem.contains_path?(source_location)
end

sig { params(constant: Module, strict: T::Boolean).returns(T::Boolean) }
Expand Down Expand Up @@ -935,11 +950,13 @@ def superclass_of(constant)
Class.instance_method(:superclass).bind(constant).call
end

SignatureBlockError = Class.new(StandardError)

sig { params(method: T.any(UnboundMethod, Method)).returns(T.untyped) }
def signature_of(method)
T::Private::Methods.signature_for_method(method)
rescue LoadError, StandardError
nil
raise SignatureBlockError
end

sig { params(constant: T::Types::Base).returns(String) }
Expand Down
14 changes: 0 additions & 14 deletions spec/tapioca/compilers/symbol_table_compiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2306,17 +2306,11 @@ class Bar < ::T::Struct
prop :bar, String
const :baz, T::Hash[String, T.untyped]
prop :quux, T.untyped, default: T.unsafe(nil)
class << self
def inherited(s); end
end
end
class Baz
abstract!
def initialize(*args, &blk); end
sig { abstract.void }
def do_it; end
end
Expand Down Expand Up @@ -2452,10 +2446,6 @@ class Foo < ::T::Struct
Elem = type_member
const :foo, T.untyped
class << self
def inherited(s); end
end
end
RBI

Expand Down Expand Up @@ -2530,10 +2520,6 @@ class Foo < ::T::Struct
prop :l, T::Array[Foo], default: T.unsafe(nil)
prop :m, T::Hash[Foo, Foo], default: T.unsafe(nil)
prop :n, Foo, default: T.unsafe(nil)
class << self
def inherited(s); end
end
end
RBI

Expand Down

0 comments on commit 593bfa7

Please sign in to comment.