[ruby-core:80425] [Ruby trunk Bug#13358] OpenStruct overriding allocate

Issue #13358 has been updated by Eregon (Benoit Daloze).


nobu (Nobuyoshi Nakada) wrote:
> Eregon (Benoit Daloze) wrote:
> > But my main issue with this fix is it only addresses a specific use-case and not the general issue:
> > `#respond_to?` should work on any object, even uninitialized and just `#allocate`-d.
> > `Kernel#respond_to_missing?` works on any object, but `OpenStruct#respond_to_missing?` does not currently.
> 
> I can't get your point.
> `OpenStruct#respond_to?` works as others, and `OpenStruct#respond_to_missing?` too.
> Anyway `Kernel#respond_to_missing?` is private and not to be used directly.

No, `OpenStruct#respond_to?` does not:

`p Class.instance_method(:allocate).bind(OpenStruct).call.respond_to?(:foo)` # or rb_obj_alloc() in C
Trunk:
.../ruby-trunk/lib/ruby/2.5.0/ostruct.rb:201:in `respond_to_missing?': undefined method 'key?' for nil:NilClass (NoMethodError)
Ruby 2.2:
false

> > For instance, `Class.instance_method(:allocate).bind(OpenStruct).call.respond_to?(:foo)` breaks.
> 
> Why bypass overridden methods?

Because it is possible, and I don't see why that should raise an error.
Class#allocate can allocate any object except OpenStruct?
That seems a not needed exception.

It's also what rb_obj_alloc() does.

> > I will commit my patch tomorrow unless you object.
> 
> I object.
> 
> > It is more robust and has no significant downside.
> 
> No merit.

Robustness of Ruby code has no merit?

The *reason* of this bug is the previous workaround.
Now it's a workaround on top of another workaround,
using meta-programming just to avoid a nil check,
and incorrect if #allocate is not called dynamically.
It also breaks the contract of #allocate to not initialize the instance,
and still lets OpenStruct#respond_to? raise an error when Kernel#respond_to? never does.

I attach my proposed patch, please have a look and consider its merits.

----------------------------------------
Bug #13358: OpenStruct overriding allocate
https://bugs.ruby-lang.org/issues/13358#change-63918

* Author: sitter (Harald Sitter)
* Status: Closed
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-linux]
* Backport: 2.2: DONTNEED, 2.3: DONE, 2.4: DONTNEED
----------------------------------------
In https://github.com/ruby/ruby/commit/15960b37e82ba60455c480b1c23e1567255d3e05 OpenStruct gained

~~~ruby
  class << self # :nodoc:
    alias allocate new
  end
~~~

Which is rather severely conflicting with expected behavior as `Class.allocate` is meant to [not call initialize](http://ruby-doc.org/core-2.4.0/Class.html#method-i-allocate). So, in fact, the change made `allocate` of `OpenStruct` do what `allocate` is asserting not to do :-/

For `OpenStruct` itself that isn't that big a deal, for classes inheriting from `OpenStruct` it breaks `allocate` though.

Example:

~~~ruby
require 'ostruct'

class A < OpenStruct
  def initialize(x, y = {})
    super(y)
  end
end

A.allocate
~~~

As `allocate` is alias'd to `new` in `OpenStruct` this will attempt to initialize `A` which will raise an `ArgumentError` because `A` cannot be initialized without arguments.

~~~
$ ruby x.rb
x.rb:4:in `initialize': wrong number of arguments (given 0, expected 1..2) (ArgumentError)
        from x.rb:9:in `new'
        from x.rb:9:in `<main>'
~~~

OpenStruct at the very least should document the fact that its allocate is behaving differently.
Ideally, `OpenStruct` should not alias allocate at all.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>