-
-
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]}" |
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.
Hmm...a simple example with just class A; end fals in Ruby 1.9 for me. Is this using DelegateClass properly?