-
-
Save xman/959291 to your computer and use it in GitHub Desktop.
diff --git a/src/org/jruby/ext/ffi/Struct.java b/src/org/jruby/ext/ffi/Struct.java | |
index e6125f5..a851305 100644 | |
--- a/src/org/jruby/ext/ffi/Struct.java | |
+++ b/src/org/jruby/ext/ffi/Struct.java | |
@@ -13,6 +13,7 @@ import org.jruby.exceptions.RaiseException; | |
import org.jruby.runtime.ObjectAllocator; | |
import org.jruby.runtime.ThreadContext; | |
import org.jruby.runtime.builtin.IRubyObject; | |
+import static org.jruby.runtime.Visibility.*; | |
@JRubyClass(name="FFI::Struct", parent="Object") | |
public class Struct extends RubyObject implements StructLayout.Storage { | |
@@ -115,7 +116,7 @@ public class Struct extends RubyObject implements StructLayout.Storage { | |
return new Struct(runtime, (RubyClass) klass, getStructLayout(runtime, klass), ptr); | |
} | |
- @JRubyMethod(name = "initialize") | |
+ @JRubyMethod(name = "initialize", visibility = PRIVATE) | |
public IRubyObject initialize(ThreadContext context) { | |
memory = MemoryPointer.allocate(context.getRuntime(), layout.getSize(), 1, true); | |
@@ -123,7 +124,7 @@ public class Struct extends RubyObject implements StructLayout.Storage { | |
return this; | |
} | |
- @JRubyMethod(name = "initialize") | |
+ @JRubyMethod(name = "initialize", visibility = PRIVATE) | |
public IRubyObject initialize(ThreadContext context, IRubyObject ptr) { | |
if (!(ptr instanceof AbstractMemory)) { | |
@@ -141,7 +142,7 @@ public class Struct extends RubyObject implements StructLayout.Storage { | |
return this; | |
} | |
- @JRubyMethod(name = "initialize_copy") | |
+ @JRubyMethod(name = "initialize_copy", visibility = PRIVATE) | |
public IRubyObject initialize_copy(ThreadContext context, IRubyObject other) { | |
if (other == this) { | |
return this; |
require 'ffi' | |
require 'delegate' | |
class A < FFI::Struct | |
layout( | |
:x, :int, | |
:y, :int, | |
) | |
end | |
class B < DelegateClass(A) | |
def initialize | |
@s = A.new | |
super(@s) | |
end | |
end | |
b = B.new | |
puts "B.new returns nil? #{b.nil?}" | |
b[:x] = 10 | |
b[:y] = 20 | |
puts "b[:x] = #{b[:x]}" | |
puts "b[:y] = #{b[:y]}" |
It looks like the getobj call in delegate.rb is producing nil.
Hmm...a simple example with just class A; end fals in Ruby 1.9 for me. Is this using DelegateClass properly?
I learnt this from the sample codes here: http://www.ruby-doc.org/stdlib/libdoc/delegate/rdoc/index.html
It's how tempfile.rb is doing with File.
But the good news is, manually calling setobj(@s) in the initialize method gets it running fine :)
So the setobj() is missing in the DelegateClass constructor? :p
FYI: The MRI Ruby uses the ffi gem, while the JRuby uses the built-in ffi.
After much tracing, I finally found the fundamental logic to the problem. The DelegateClass(A) returns a class with methods to delegation blocks for most methods found in A.instance_methods, and initialize() is in the list. As a result, the initialize() of the superclass is not called, consequently setobj() is not invoked too. MRI Ruby doesn't have this problem as its A.instance_methods do not include initialize().
Hunting down further, I found that the file jruby-1.6.1/src/org/jruby/ext/ffi/Struct.java does not define the visibility of the initialize method. By applying the patch above, I managed to remove initialize() from the A.instance_methods list. But I have no idea if the visibility is left out intentionally. Then, the DelegateClass(A) would work as expected.
If you think this is a bug, I'll file a bug report :)
Nicely done! I noticed the initialize thing too, but then in reproducing it I walked away from the culprit: Struct. I think you are exactly right, so file the bug with your patch and workaround and we will fix for 1.6.2.
Referencing the A.instance_methods.grep /init/ in MRI Ruby 1.9.2, you actually get this:
initialize_dup
initialize_clone
while JRuby gives this:
initialize
initialize_copy
Perhaps there is something need to be done to make these consistent :) Otherwise, any dynamic classes rely on these will break easily :p
The patch above doesn't remove the initialize_copy() from the list, may be I'll do that to make it consistent with MRI Ruby.
It's weird that in JRuby 1.6.1, the b.nil? is false, yet the error message seems to suggest b is nil :p