BasicObject.respond_to? raises a "undefined method `respond_to_missing?'"

Details

Type: Bug
Status: Resolved
Priority: Minor
Resolution: Fixed
Affects Version/s: JRuby 1.6.7, JRuby 1.7.0.pre1
Fix Version/s: JRuby 1.7.0.pre2
Component/s: Core Classes/Modules
Labels:
None

Number of attachments :
0

Description

class X < BasicObject
  def respond_to?(meth); false; end
end

X.new.respond_to?(:hello)

# => NoMethodError: undefined method `respond_to_missing?'

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Charles Oliver Nutter added a comment - 26/Jun/12 3:43 PM

Inconsistent on Java 7 as well. It's either doing it right or failing to do it at all.

1.9.3 does not raise this error.

system ~/projects/jruby/tmp $ pickjdk 1
New JDK: 1.6.0.jdk

system ~/projects/jruby/tmp $ jruby blah.rb
NoMethodError: undefined method `respond_to_missing?' for #<X:0x5dbbd583>
  (root) at blah.rb:5

system ~/projects/jruby/tmp $ pickjdk 2
New JDK: 1.7.0.jdk

system ~/projects/jruby/tmp $ jruby blah.rb

system ~/projects/jruby/tmp $

Show

Charles Oliver Nutter added a comment - 26/Jun/12 3:43 PM Inconsistent on Java 7 as well. It's either doing it right or failing to do it at all. 1.9.3 does not raise this error. system ~/projects/jruby/tmp $ pickjdk 1 New JDK: 1.6.0.jdk system ~/projects/jruby/tmp $ jruby blah.rb NoMethodError: undefined method `respond_to_missing?' for #<X:0x5dbbd583> (root) at blah.rb:5 system ~/projects/jruby/tmp $ pickjdk 2 New JDK: 1.7.0.jdk system ~/projects/jruby/tmp $ jruby blah.rb system ~/projects/jruby/tmp $

Hide

Permalink

Charles Oliver Nutter added a comment - 27/Jun/12 11:13 AM

A patch to remove the logic from RespondToCallSite does not appear to work. It passes test/unit tests in JRuby, MRI, and Rubicon, but does not pass RubySpec.

It seems that our RespondToCallSite is still flawed for how it handles 1.9 mode.

diff --git a/src/org/jruby/runtime/callsite/RespondToCallSite.java b/src/org/jruby/runtime/callsite/RespondToCallSite.java
index ed101ab..a648ce4 100644
--- a/src/org/jruby/runtime/callsite/RespondToCallSite.java
+++ b/src/org/jruby/runtime/callsite/RespondToCallSite.java
@@ -44,13 +44,7 @@ public class RespondToCallSite extends NormalCachingCallSite {
             if (strName.equals(tuple.name) && tuple.checkVisibility) return tuple.respondsTo;
         }
         // go through normal call logic, which will hit overridden cacheAndCall
-        IRubyObject respond = super.call(context, caller, self, name);
-
-        if (!respond.isTrue() && context.getRuntime().is1_9()) {
-            respond = self.callMethod(context, "respond_to_missing?", new IRubyObject[]{name, context.getRuntime().getFalse()});
-            respond = context.getRuntime().newBoolean(respond.isTrue());
-        }
-        return respond;
+        return super.call(context, caller, self, name);
     }
 
     @Override
@@ -62,13 +56,7 @@ public class RespondToCallSite extends NormalCachingCallSite {
             if (strName.equals(tuple.name) && !bool.isTrue() == tuple.checkVisibility) return tuple.respondsTo;
         }
         // go through normal call logic, which will hit overridden cacheAndCall
-        IRubyObject respond = super.call(context, caller, self, name, bool);
-
-        if (!respond.isTrue() && context.getRuntime().is1_9()) {
-            respond = self.callMethod(context, "respond_to_missing?", new IRubyObject[]{name, bool});
-            respond = context.getRuntime().newBoolean(respond.isTrue());
-        }
-        return respond;
+        return super.call(context, caller, self, name, bool);
     }
 
     @Override

Show

Charles Oliver Nutter added a comment - 27/Jun/12 11:13 AM A patch to remove the logic from RespondToCallSite does not appear to work. It passes test/unit tests in JRuby, MRI, and Rubicon, but does not pass RubySpec. It seems that our RespondToCallSite is still flawed for how it handles 1.9 mode. diff --git a/src/org/jruby/runtime/callsite/RespondToCallSite.java b/src/org/jruby/runtime/callsite/RespondToCallSite.java index ed101ab..a648ce4 100644 --- a/src/org/jruby/runtime/callsite/RespondToCallSite.java +++ b/src/org/jruby/runtime/callsite/RespondToCallSite.java @@ -44,13 +44,7 @@ public class RespondToCallSite extends NormalCachingCallSite { if (strName.equals(tuple.name) && tuple.checkVisibility) return tuple.respondsTo; } // go through normal call logic, which will hit overridden cacheAndCall - IRubyObject respond = super.call(context, caller, self, name); - - if (!respond.isTrue() && context.getRuntime().is1_9()) { - respond = self.callMethod(context, "respond_to_missing?", new IRubyObject[]{name, context.getRuntime().getFalse()}); - respond = context.getRuntime().newBoolean(respond.isTrue()); - } - return respond; + return super.call(context, caller, self, name); } @Override @@ -62,13 +56,7 @@ public class RespondToCallSite extends NormalCachingCallSite { if (strName.equals(tuple.name) && !bool.isTrue() == tuple.checkVisibility) return tuple.respondsTo; } // go through normal call logic, which will hit overridden cacheAndCall - IRubyObject respond = super.call(context, caller, self, name, bool); - - if (!respond.isTrue() && context.getRuntime().is1_9()) { - respond = self.callMethod(context, "respond_to_missing?", new IRubyObject[]{name, bool}); - respond = context.getRuntime().newBoolean(respond.isTrue()); - } - return respond; + return super.call(context, caller, self, name, bool); } @Override

Hide

Permalink

Charles Oliver Nutter added a comment - 27/Jun/12 1:23 PM

Removing the use of RespondToCallSite fixes this. I'm looking to make sure RespondToCallSite matches the 1.9 logic.

Show

Charles Oliver Nutter added a comment - 27/Jun/12 1:23 PM Removing the use of RespondToCallSite fixes this. I'm looking to make sure RespondToCallSite matches the 1.9 logic.

Hide

Permalink

Charles Oliver Nutter added a comment - 27/Jun/12 1:41 PM

Ok, the problem here is that our respond_to? caching logic unconditionally tries to call respond_to_missing? on a false result, even when the false result is coming from a user-defined method. This is incorrect. It should only attempt to try to call respond_to_missing? if the built-in respond_to? would return a false result.

Because repairing this behavior would require making the caching logic more complicated (due to the need to check if respond_to_missing? is the default impl or not) I'm going to just disable caching of respond_to? results in 1.9 mode. We'll need to revisit this.

Show

Charles Oliver Nutter added a comment - 27/Jun/12 1:41 PM Ok, the problem here is that our respond_to? caching logic unconditionally tries to call respond_to_missing? on a false result, even when the false result is coming from a user-defined method. This is incorrect. It should only attempt to try to call respond_to_missing? if the built-in respond_to? would return a false result. Because repairing this behavior would require making the caching logic more complicated (due to the need to check if respond_to_missing? is the default impl or not) I'm going to just disable caching of respond_to? results in 1.9 mode. We'll need to revisit this.

Hide

Permalink

Charles Oliver Nutter added a comment - 27/Jun/12 2:18 PM

commit 186b650c45b2a43bc45eb53b8866fab40cd8072a
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Wed Jun 27 14:14:52 2012 -0500

    Fix JRUBY-6740
    
    BasicObject.respond_to? raises a "undefined method `respond_to_missing?'"
    
    The caching logic for respond_to? was unconditionally calling
    respond_to_missing? even for custom respond_to? methods. It should
    only be calling it for the builtin respond_to? logic. However, in
    order to make it work properly, respond_to_missing? logic would
    need to be added to the caching logic, meaning we would need to
    additionally check whether respond_to_missing? was also builtin
    and invalidate appropriately. Due to the complexity of adding
    another dimension to respond_to? caching, I'm just disabling the
    caching logic altogether for 1.9 mode and have filed JRUBY-6749
    to track us getting it reenabled.
    
    FWIW, indy dispatch does not do any respond_to? caching for either
    1.8 or 1.9 mode, and was unaffected by this bug.

Show

Charles Oliver Nutter added a comment - 27/Jun/12 2:18 PM commit 186b650c45b2a43bc45eb53b8866fab40cd8072a Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Jun 27 14:14:52 2012 -0500 Fix JRUBY-6740 BasicObject.respond_to? raises a "undefined method `respond_to_missing?'" The caching logic for respond_to? was unconditionally calling respond_to_missing? even for custom respond_to? methods. It should only be calling it for the builtin respond_to? logic. However, in order to make it work properly, respond_to_missing? logic would need to be added to the caching logic, meaning we would need to additionally check whether respond_to_missing? was also builtin and invalidate appropriately. Due to the complexity of adding another dimension to respond_to? caching, I'm just disabling the caching logic altogether for 1.9 mode and have filed JRUBY-6749 to track us getting it reenabled. FWIW, indy dispatch does not do any respond_to? caching for either 1.8 or 1.9 mode, and was unaffected by this bug.

People

Assignee:

Charles Oliver Nutter

Reporter:

Bernard Lambeau

Vote (0)

Watch (4)

Dates

Created:

22/Jun/12 6:17 AM

Updated:

27/Jun/12 2:18 PM

Resolved:

27/Jun/12 2:18 PM