Fix for retrieving UnboundMethod parameters #2232

Merged
merged 5 commits into from Mar 27, 2013
@@ -52,6 +52,10 @@ def splat
@block_env.compiled_code.splat
end
+ def block_index
+ @block_env.compiled_code.block_index
+ end
+
def file
@block_env.file
end
@@ -84,6 +84,10 @@ def arity
end
end
+ def parameters
+ @compiled_code.parameters
+ end
+
def file
@compiled_code.file
end
@@ -127,6 +131,10 @@ def ==(other)
@block_env == other.block_env
end
+
+ def parameters
+ @block_env.parameters
+ end
end
end
end
@@ -75,6 +75,7 @@ def equivalent_body?(other)
@required_args == other.required_args and
@total_args == other.total_args and
@splat == other.splat and
+ @block_index == other.block_index and
@literals == other.literals and
@file == other.file and
@local_names == other.local_names
@@ -92,6 +93,24 @@ def local_slot(name)
end
##
+ # Stores the index of the block argument in the metadata storage.
+ #
+ # @param [Fixnum] position
+ #
+ def block_index=(position)
+ if position
+ add_metadata(:block_index, position)
+ end
+ end
+
+ ##
+ # @return [Fixnum|NilClass]
+ #
+ def block_index
+ return get_metadata(:block_index)
+ end
+
+ ##
# Return a human readable interpretation of this method.
#
# @return [String]
@@ -491,24 +510,30 @@ def get_metadata(key)
##
# For Method#parameters
def parameters
+ params = []
+
+ return params unless respond_to?(:local_names)
+
m = required_args - post_args
o = m + total_args - required_args
p = o + post_args
p += 1 if splat
- local_names.each_with_index.map do |name, i|
+ local_names.each_with_index do |name, i|
if i < m
- [:req, name]
+ params << [:req, name]
elsif i < o
- [:opt, name]
+ params << [:opt, name]
elsif splat == i
- [:rest, name]
+ params << [:rest, name]
elsif i < p
- [:req, name]
- else
- [:block, name]
+ params << [:req, name]
+ elsif block_index == i
+ params << [:block, name]
end
end
+
+ return params
end
##
View
@@ -262,26 +262,7 @@ def source_location
end
def parameters
- return [] unless @executable.respond_to? :local_names
-
- m = @executable.required_args - @executable.post_args
- o = m + @executable.total_args - @executable.required_args
- p = o + @executable.post_args
- p += 1 if @executable.splat
-
- @executable.local_names.each_with_index.map do |name, i|
- if i < m
- [:req, name]
- elsif i < o
- [:opt, name]
- elsif @executable.splat == i
- [:rest, name]
- elsif i < p
- [:req, name]
- else
- [:block, name]
- end
- end
+ return @executable.respond_to?(:parameters) ? @executable.parameters : []
@ryoqun

ryoqun Mar 25, 2013

Member

Thanks for contributing! I think this is suspicious for the sudden CI failure. I'm not absolutely sure, but probably, I think @eXecutable can be some thing other than CompiledCode? How about changing this empty array with some thing like [:foo, :bar] to check the empty array of actual returned value really came from here in CI? If I'm wrong, I'll test your pull request locally.

@YorickPeterse

YorickPeterse Mar 25, 2013

Owner

The weird thing is that even with a clean working directory (before I committed these changes) the particular test would fail. This would indicate that something else is affecting it, but I have no idea why that would be the case all of a sudden.

@dbussink

dbussink Mar 25, 2013

Owner

I think you are still confused on tags work. If you run ./bin/mspec path/to/spec/file.rb directly, it will ignore tags. That means it will run all the specs, even the specs that are known to fail. The CI process does consider the tags, so you don't see the failure then. You won't see the failure either when running 'rake', since that just runs the CI task.

For you now, you can safely ignore this failing spec, since that's for a totally different issue, not related to the problem this pull request tries to fix.

@YorickPeterse

YorickPeterse Mar 25, 2013

Owner

Well that's the thing, the failures do occur both on Travis and when running rake.

@dbussink

dbussink Mar 25, 2013

Owner

Ah ok, then yeah, they do seem related to the changes here then. I don't see those currently for master or here locally, so there must be something different for you then indeed.

@YorickPeterse

YorickPeterse Mar 25, 2013

Owner

The odd thing is that when I reset my changes they still failed, which, according to Travis should not be possible. I'll take a closer look at this this evening.

@ryoqun

ryoqun Mar 25, 2013

Member

As I guessed, @executable can be BlockEnvironment::AsMethod along side CompiledCode. I fixed that at my branch based on your branch with this commit.

Could you rebase --interactive this, while unifying it into your refactoring commit?

end
def owner
@@ -292,12 +292,13 @@ def bytecode(g, recv)
class FormalArguments < Node
attr_accessor :names, :required, :optional, :defaults, :splat
- attr_reader :block_arg
+ attr_reader :block_arg, :block_index
def initialize(line, args, defaults, splat)
@line = line
@defaults = nil
@block_arg = nil
+ @block_index = nil
if defaults
defaults = DefaultArguments.new line, defaults
@@ -324,6 +325,8 @@ def initialize(line, args, defaults, splat)
def block_arg=(node)
@names << node.name
+
+ @block_index = @names.length - 1
@block_arg = node
end
@@ -410,6 +413,7 @@ def initialize(line, required, optional, splat, post, block)
@defaults = nil
@block_arg = nil
@splat_index = nil
+ @block_index = nil
@required = []
names = []
@@ -456,6 +460,7 @@ def initialize(line, required, optional, splat, post, block)
if block
@block_arg = BlockArgument.new line, block
names << block
+ @block_index = names.length - 1
end
@splat = splat
View
@@ -76,6 +76,7 @@ def new_generator(g, name, arguments=nil)
meth.post_args = arguments.post_args
meth.total_args = arguments.total_args
meth.splat_index = arguments.splat_index
+ meth.block_index = arguments.block_index
end
meth
@@ -266,6 +266,7 @@ def initialize
@splat_index = nil
@local_names = nil
+ @block_index = nil
@local_count = 0
@state = []
@@ -282,7 +283,8 @@ def initialize
attr_accessor :break, :redo, :next, :retry, :file, :name,
:required_args, :post_args, :total_args, :splat_index,
:local_count, :local_names, :primitive, :for_block,
- :current_block, :detected_args, :detected_locals
+ :current_block, :detected_args, :detected_locals,
+ :block_index
def execute(node)
node.bytecode self
@@ -320,6 +322,7 @@ def package(klass)
code.post_args = @post_args
code.total_args = @total_args
code.splat = @splat_index
+ code.block_index = @block_index
code.local_count = @local_count
code.local_names = @local_names.to_tuple if @local_names
@@ -2,10 +2,10 @@ module MethodSpecs
class SourceLocation
- def location # This needs to be on this line
+ def location # This needs to be on this line
:location # for the spec to pass
end
-
+
def redefined
:first
end
@@ -17,19 +17,28 @@ def redefined
def original
end
- alias :aka :original
+ alias :aka :original
end
class Methods
def zero; end
+
+ def zero_with_locals
+ number = 10
+ end
+
def one_req(a); end
def two_req(a, b); end
def zero_with_block(&block); end
def one_req_with_block(a, &block); end
def two_req_with_block(a, b, &block); end
+ def zero_with_block_and_locals(&block)
+ number = 10
+ end
+
def one_opt(a=nil); end
def one_req_one_opt(a, b=nil); end
def one_req_two_opt(a, b=nil, c=nil); end
@@ -7,6 +7,12 @@
MethodSpecs::Methods.instance_method(:zero).parameters.should == []
end
+ it "returns an empty Array when the method expects no arguments but does have local variables" do
+ m = MethodSpecs::Methods.instance_method(:zero_with_locals)
+
+ m.parameters.should == []
+ end
+
it "returns [[:req,:name]] for a method expecting one required argument called 'name'" do
MethodSpecs::Methods.instance_method(:one_req).parameters.should == [[:req,:a]]
end
@@ -21,6 +27,12 @@
m.parameters.should == [[:block,:block]]
end
+ it "returns [[:block,:block]] for a method expecting one block argument with local variables" do
+ m = MethodSpecs::Methods.instance_method(:zero_with_block_and_locals)
+
+ m.parameters.should == [[:block,:block]]
+ end
+
it "returns [[:req,:a],[:block,:b] for a method expecting a required argument ('a') and a block argument ('b')" do
m = MethodSpecs::Methods.instance_method(:one_req_with_block)
m.parameters.should == [[:req,:a], [:block,:block]]
@@ -126,4 +138,4 @@
m.parameters.should == [[:req,:a],[:req,:b],[:opt,:c],[:rest,:d],[:block,:block]]
end
-end
+end
@@ -91,7 +91,7 @@ def send_vcall(meth)
attr_accessor :stream, :ip, :redo, :break, :next, :retry,
:name, :file, :line, :primitive, :for_block,
:required_args, :post_args, :total_args, :splat_index,
- :local_count, :local_names
+ :local_count, :local_names, :block_index
def initialize