Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: eed14f749028
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: db484cc30a54
Choose a head ref
  • 3 commits
  • 3 files changed
  • 2 contributors

Commits on Apr 2, 2015

  1. Support respond_to_missing? in respondsTo() lookups 

    RubyBasicObject assumes that if respond_to? is undefined, it can just fast-track
method lookups. We have to also check for respond_to_missing?; failure to do so causes
Javaland respondsTo() calls to fail when an object defines respond_to_missing? (despite
the documentation's admonishment to not use it)

The practical case for this is ActiveSupport::Duration in ActiveSupport 4.2.1
    @cheald
    cheald committed Apr 2, 2015
    Copy the full SHA
    fe8c87a View commit details

Commits on Apr 4, 2015

  1. Don't depend on method.getName() to have the right name for respond_t… 

    …o/respond_to_missing
    @cheald
    cheald committed Apr 4, 2015
    Copy the full SHA
    7708f4c View commit details

Commits on Apr 6, 2015

  1. Merge pull request #2797 from cheald/fix-respond-to-missing 

    Support respond_to_missing? in respondsTo() lookups
    @enebo
    enebo committed Apr 6, 2015
    Copy the full SHA
    db484cc View commit details
Showing with 42 additions and 13 deletions.
  1. +20 −12 core/src/main/java/org/jruby/RubyBasicObject.java
  2. +1 −1 core/src/main/java/org/jruby/RubyNumeric.java
  3. +21 −0 test/jruby/test_respond_to.rb
32 changes: 20 additions & 12 deletions core/src/main/java/org/jruby/RubyBasicObject.java
View file
Original file line number Diff line number Diff line change
@@ -583,33 +583,41 @@ public RubyClass getType() {

/**
* Does this object respond to the specified message? Uses a
* shortcut if it can be proved that respond_to? haven't been
* overridden.
* shortcut if it can be proved that respond_to? and respond_to_missing?
* haven't been overridden.
*/
@Override
public final boolean respondsTo(String name) {
Ruby runtime = getRuntime();

DynamicMethod method = getMetaClass().searchMethod("respond_to?");
if(method.equals(runtime.getRespondToMethod())) {
DynamicMethod respondTo = getMetaClass().searchMethod("respond_to?");
DynamicMethod respondToMissing = getMetaClass().searchMethod("respond_to_missing?");

if(respondTo.equals(runtime.getRespondToMethod()) && respondToMissing.equals(runtime.getRespondToMissingMethod())) {
// fastest path; builtin respond_to? which just does isMethodBound
return getMetaClass().isMethodBound(name, false);
} else if (!method.isUndefined()) {
// medium path, invoke user's respond_to? if defined
} else if (!(respondTo.isUndefined() && respondToMissing.isUndefined())) {
// medium path, invoke user's respond_to?/respond_to_missing? if defined
DynamicMethod method;
String methodName;
if (respondTo.isUndefined()) {
method = respondToMissing;
methodName = "respond_to_missing?";
} else {
method = respondTo;
methodName = "respond_to?";
}

// We have to check and enforce arity
Arity arity = method.getArity();
ThreadContext context = runtime.getCurrentContext();
if (arity.isFixed() && arity.required() == 1) {
return method.call(context, this, metaClass, "respond_to?", runtime.newSymbol(name)).isTrue();
return method.call(context, this, metaClass, methodName, runtime.newSymbol(name)).isTrue();
} else if (arity.isFixed() && arity.required() != 2) {
throw runtime.newArgumentError("respond_to? must accept 1 or 2 arguments (requires " + arity.getValue() + ")");
} else {

throw runtime.newArgumentError(methodName + " must accept 1 or 2 arguments (requires " + arity.getValue() + ")");
}

return method.call(context, this, metaClass, "respond_to?", runtime.newSymbol(name), runtime.newBoolean(true)).isTrue();

return method.call(context, this, metaClass, methodName, runtime.newSymbol(name), runtime.newBoolean(true)).isTrue();
} else {
// slowest path, full callMethod to hit method_missing if present, or produce error
return callMethod(runtime.getCurrentContext(), "respond_to?", runtime.newSymbol(name)).isTrue();
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyNumeric.java
View file
Original file line number Diff line number Diff line change
@@ -447,7 +447,7 @@ protected final RubyArray doCoerce(ThreadContext context, IRubyObject other, boo
Ruby runtime = context.runtime;
IRubyObject result;

IRubyObject savedError = runtime.getGlobalVariables().get("$!"); // Svae $!
IRubyObject savedError = runtime.getGlobalVariables().get("$!"); // Save $!

if (!other.respondsTo("coerce")) {
if (err) {
21 changes: 21 additions & 0 deletions test/jruby/test_respond_to.rb
View file
Original file line number Diff line number Diff line change
@@ -107,3 +107,24 @@ def test_respond_to_check_can_trigger_method_missing
end
end
end

class TestRespondToMissingFastPath < Test::Unit::TestCase
class Duration
def initialize
@value = 10
end

def respond_to_missing?(method, include_private=false)
@value.respond_to?(method, include_private)
end

def method_missing(method, *args, &block)
@value.send(method, *args, &block)
end
end

def test_respond_to_doesnt_fastpath_if_respond_to_missing_exists
obj = Duration.new
assert(10 * obj == 100)
end
end