Bug #11326
closedDefining a writer as a Struct member allowed?
Description
Hi,
yesterday I've stumbled on weird behavior defining writers as Struct members.
While this code works on MRI it breaks on JRuby and Rubinius:
Struct.new(:x=).new(nil).x = 23
On JRuby it fails with:
ArgumentError: wrong number of arguments calling `x=` (1 for 0)
from (irb):1:in `evaluate'
from org/jruby/RubyKernel.java:1111:in `eval'
from org/jruby/RubyKernel.java:1511:in `loop'
from org/jruby/RubyKernel.java:1274:in `catch'
from org/jruby/RubyKernel.java:1274:in `catch'
from /home/ps/.rvm/rubies/jruby-1.7.20/bin/irb:13:in `(root)'
On Rubinius it gives:
ArgumentError: given 1, expected 0
from (irb):1
from kernel/common/block_environment.rb:53:in `call_on_instance'
from kernel/common/eval.rb:176:in `eval'
from kernel/common/kernel.rb:510:in `loop'
from kernel/bootstrap/proc.rb:20:in `call'
from kernel/common/throw_catch.rb:30:in `catch'
from kernel/common/throw_catch.rb:8:in `register'
from kernel/common/throw_catch.rb:29:in `catch'
from kernel/bootstrap/proc.rb:20:in `call'
from kernel/common/throw_catch.rb:30:in `catch'
from kernel/common/throw_catch.rb:8:in `register'
from kernel/common/throw_catch.rb:29:in `catch'
from /home/ps/.rvm/gems/rbx-2.5.2@global/gems/rubysl-irb-2.1.1/bin/irb:12:in `__script__'
from kernel/common/kernel.rb:497:in `load'
from /home/ps/.rvm/gems/rbx-2.5.2@global/bin/irb:23:in `__script__'
from kernel/delta/code_loader.rb:66:in `load_script'
from kernel/delta/code_loader.rb:152:in `load_script'
from kernel/loader.rb:655:in `script'
from kernel/loader.rb:809:in `main'
I've reported this as a bug to the JRuby team at https://github.com/jruby/jruby/issues/3097
Charles rejected it and said:
I think I'm going to call this a bug in MRI and ask you to re-file it with them. I believe the
:x=
should be kicked out as an invalid attribute name rather than defined as a write-only attribute. I believe the latter behavior is happening only accidentally because of how MRI detects that you're defining an=
method.
So here I am.
I don't have a strong opinion whether defining a writer member should be possible or not.
It would be nice to have one single behavior across the 3 major Ruby implementations :)
What do you think?
Kind regards,
Peter
Files
Updated by normalperson (Eric Wong) over 9 years ago
peter-rl@suschlik.de wrote:
I don't have a strong opinion whether defining a writer member should
be possible or not.
I suggest banning it (barely-tested patch):
--- a/struct.c
+++ b/struct.c
@@ -12,6 +12,7 @@
#include "internal.h"
#include "vm_core.h"
#include "id.h"
+#include "symbol.h"
/* only for struct[:field] access */
#define AREF_HASH_THRESHOLD (10)
@@ -510,6 +511,10 @@ rb_struct_s_def(int argc, VALUE *argv, VALUE klass)
rest = rb_ary_tmp_new(argc);
for (i=0; i<argc; i++) {
id = rb_to_id(argv[i]);
+ if (is_attrset_id(id)) {
+ rb_raise(rb_eArgError, "invalid struct member: %+"PRIsVALUE,
+ argv[i]);
+ }
RARRAY_ASET(rest, i, ID2SYM(id));
rb_ary_set_len(rest, i+1);
}
$ ./ruby -e 'Struct.new(:foo=)'
-e:1:in `new': invalid struct member: :foo= (ArgumentError)
from -e:1:in `<main>'
Updated by nobu (Nobuyoshi Nakada) over 9 years ago
- Description updated (diff)
Eric Wong wrote:
I suggest banning it (barely-tested patch):
+1
Updated by normalperson (Eric Wong) over 9 years ago
nobu@ruby-lang.org wrote:
Eric Wong wrote:
I suggest banning it (barely-tested patch):
+1
OK with matz to commit my patch? (+ tests)?
Also, what about '?' and '!' in field names?
Can we continue to allow those?
I guess the following work in existing code:
s[:foo?] = :bar
s[:foo!] = :bar
Updated by jeremyevans0 (Jeremy Evans) over 5 years ago
This issue still exists in the master branch. Eric's patch no longer applies, but attached is a similar approach that works and includes a test. However, maybe there was a reason this wasn't committed originally?
Updated by matz (Yukihiro Matsumoto) about 5 years ago
Accepted.
Matz.
Updated by jeremyevans (Jeremy Evans) about 5 years ago
- Status changed from Open to Closed
Applied in changeset git|e51dca2596db9567bd4d698b18b4d300575d3881.
Disallow use of attrset symbols as Struct members
Fixes [Bug #11326]