[ruby-core:69849] [Ruby trunk - Bug #11326] [Open] Defining a writer as a Struct member allowed?

[ruby-core:69849] [Ruby trunk - Bug #11326] [Open] Defining a writer as a Struct member allowed?

[peter-rl]

Issue #11326 has been reported by Peter Suschlik.

----------------------------------------
Bug #11326: Defining a writer as a Struct member allowed?
https://bugs.ruby-lang.org/issues/11326

* Author: Peter Suschlik
* Status: Open
* Priority: Normal
* Assignee: 
* ruby -v: 2.2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
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:

```ruby
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



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

[ruby-core:69850] Re: [Ruby trunk - Bug #11326] [Open] Defining a writer as a Struct member allowed?

[Eric Wong]

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>'

[ruby-core:69855] [Ruby trunk - Bug #11326] Defining a writer as a Struct member allowed?

[nobu]

Issue #11326 has been updated by Nobuyoshi Nakada.

Description updated

Eric Wong wrote:
> I suggest banning it (barely-tested patch):

+1

----------------------------------------
Bug #11326: Defining a writer as a Struct member allowed?
https://bugs.ruby-lang.org/issues/11326#change-53256

* Author: Peter Suschlik
* Status: Open
* Priority: Normal
* Assignee: 
* ruby -v: 2.2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
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:

```ruby
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



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

[ruby-core:69869] Re: [Ruby trunk - Bug #11326] Defining a writer as a Struct member allowed?

[Eric Wong]

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

[ruby-core:93563] [Ruby master Bug#11326] Defining a writer as a Struct member allowed?

[merch-redmine]

Issue #11326 has been updated by jeremyevans0 (Jeremy Evans).

File struct-ban-attrset-id.patch added

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?

----------------------------------------
Bug #11326: Defining a writer as a Struct member allowed?
https://bugs.ruby-lang.org/issues/11326#change-79133

* Author: splattael (Peter Suschlik)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 2.2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
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:

```ruby
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--------------------------------
struct-ban-attrset-id.patch (1.22 KB)


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

[ruby-core:94641] [Ruby master Bug#11326] Defining a writer as a Struct member allowed?

[matz]

Issue #11326 has been updated by matz (Yukihiro Matsumoto).


Accepted.

Matz.

----------------------------------------
Bug #11326: Defining a writer as a Struct member allowed?
https://bugs.ruby-lang.org/issues/11326#change-81238

* Author: splattael (Peter Suschlik)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 2.2
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
----------------------------------------
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:

```ruby
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--------------------------------
struct-ban-attrset-id.patch (1.22 KB)


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