[ruby-core:103738] [Ruby master Bug#15928] Constant declaration does not conform to JIS 3017:2013

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


jeremyevans0 (Jeremy Evans) wrote in #note-6:
> If the behavior was a deliberate feature in older versions, I agree.  If the behavior was not deliberate in old versions, and changing it is just fixing a bug (as it is in this case), I disagree.

How do you know if it was deliberate?

FWIW I think it might be deliberate, I think the implementation is simpler and more consistent the way it is.
For `a.b.c, d.e.f = values` we currently evaluate `values`, then `a.b.c = values[0]`, then `d.e.f = values[1]` (correct me if I'm wrong).
That's simple and there is no need to switch back-and-forth left and right (it's just RHS, then all of LHS in order).
We need to evaluate `values` before the actual assignments of course, and assignments can be method calls so I think it's just fine to execute all the assignments on the left part together and not half of it first, then `values`, then the `foo=` methods.
The current strategy also feels more efficient, because the suggested change actually needs to keep temporary values for the results of `a.b` and `d.e` additionally.

I think the implementation in TruffleRuby shows nicely how simple multiple assignment semantics are currently:
https://github.com/oracle/truffleruby/blob/f2e97df99606ad382b4ba8f1272984ecfacb95f9/src/main/java/org/truffleruby/core/array/MultipleAssignmentNode.java#L61-L85
I think it would be significantly more complex with this change.

BTW that spec was added it seems before JIS 3017:2013, and JIS 3017:2013 has many differences with CRuby and many parts are not specified at all (e.g., class variables in singleton classes).
So differences with JIS 3017:2013 are somewhat expected (not ideal though).

> Simply because it took a long time for this bug to be fixed does not mean it should be treated differently from other bugs.

The key here is whether it would be backported.
I'm confident that if this is merged it would not be backported (too big risk of incompatibility).

From https://github.com/ruby/ruby/pull/4450#discussion_r626213475 (sorry, I should have replied only here to keep it in one place).

> If TruffleRuby/JRuby want to target Ruby 2.6/2.7/3.0 and want to make this change earlier, I don't think we should have a spec preventing it.

I'm extremely confident that JRuby, TruffleRuby and other Rubies would not change the behavior while they claim compatibility with Ruby <= 3.0.
They want maximum compatibility with an existing release of Ruby, whether the semantics are ideal/match JIS 3017:2013 or not is typically much less relevant than compatibility with that CRuby version.
The fact there have been user bug report(s) there mean some code must rely on this.

>  Keeping the spec for previous versions codifies wrong behavior.

It's your interpretation that it is wrong behavior.
The fact is Ruby <= 3.0 has that behavior and it's extremely unlikely to change (it's even impossible to change it for existing CRuby releases).

> I believe all bug fixes that break specs should be treated this way (removing broken specs)

Maybe this way to explain is clearer:
Removing the spec is equivalent to removing a test on a backport branch, it has the same effect, Ruby <= 3.0 is no longer tested for that case, and other Rubies targeting Ruby <= 3.0 too.
Of course, removing a test on a backport branch is completely unacceptable (unless it's transient and causing CI issues maybe, doesn't apply here).



----------------------------------------
Bug #15928: Constant declaration does not conform to JIS 3017:2013
https://bugs.ruby-lang.org/issues/15928#change-91834

* Author: yugui (Yuki Sonoda)
* Status: Open
* Priority: Normal
* ruby -v: ruby 2.7.0dev (2019-06-16T14:01:46Z master d4929f5185) [x86_64-darwin18]
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
The order of evaluation in constant declaration does not conform to JIS 3017:2013 11.4.2.2.3.

# Problem

Suppose that we are evaluating the following program.

```
expr::C = lhs
```

The standard seems to be requiring the following evaluation order:

1. expr
  * raise a TypeError if the value is not a kind of Module
2. lhs
3. rb_const_set(expr, :C, lhs)

However, the actual implementation evaluates in the following order

1. lhs
2. expr
3. rb_const_set(expr, :C, lhs)
  * raise a TypeError if the expr is not a kind of Module


# How to reproduce

The next program does not raise "recv" but raises "value"

```
raise("recv")::C = raise("value")
```

The next program does not raise a TypeError but raises a RuntimeError

```
A = 1
A::C = raise("value")
```

# Question
* Is this interpretation of the standard correct?
* If it is, Should we change the current behavior?
* If we shouldn't, does it mean an issue in the standard?

c.f.
* https://twitter.com/n0kada/status/1140234416175763456



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

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