From: merch-redmine@... Date: 2021-05-05T15:10:42+00:00 Subject: [ruby-core:103744] [Ruby master Bug#15928] Constant declaration does not conform to JIS 3017:2013 Issue #15928 has been updated by jeremyevans0 (Jeremy Evans). Eregon (Benoit Daloze) wrote in #note-7: > 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). You're wrong since commit:50c54d40a81bb2a4794a6be5f1861152900b4fed (bug fix for #4443). > 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). Multiple assignment order should operate like single assignment order, and evaluate left-to-right. `a.b = c` evaluates `a`, `c`, then `b=`. Multiple assignment has been fixed to work the same way. The only reason multiple assignment evaluation order was broken for so long is that it was considered difficult to fix and nobody put in the effort to do so. The same issue applies to constant evaluation order (this issue). > 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. If we are doing to do that, we should change single assignment order. But it is fairly clear based on discussion in #4443 that the desired evaluation order is left-to-right. > 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. Correct, there is a minor slowdown for evaluating left-to-right, measurable in microbenchmarks. It's unlikely to be significant in any real codebase. Obviously for constant assignment, that doesn't happen dynamically, so any slowdown is of no importance. > 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. If you want to maintain compatibility with CRuby, you'll need to change it anyway, and when doing so, you might as well support constant assignment the same way as attribute assignment. Yes, it is much more complex. I'm guessing that's the reason that #4443 stayed open for so long. > 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). Agreed. The primary reason to fix this is not for JIS compatibility, but for consistency with other assignment. > > 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). I think the key issue is whether the spec documents behavior that is intended. If the spec documents behavior that was unintended, it should be removed. > 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. I tried to research this, but unfortunately the JRuby JIRA was removed. From my experience with CRuby, people report bugs when they notice inconsistency that they think should be fixed, not because they are relying on the behavior. > > Keeping the spec for previous versions codifies wrong behavior. > > It's your interpretation that it is wrong behavior. Not just mine. @yugui and @mame also feel it is wrong, and the issue is similar to #4443, in which @matz confirms that left-to-right is the desired evaluation order. #4443 doesn't specifically discuss constant assignment, but it seems unlikely that constant assignment was deliberately different than attribute assignment. > 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). In my opinion, we shouldn't test for behavior we consider wrong. The behavior was wrong in Ruby 1.8-3.0 (maybe even earlier). Yes, it took a long time to fix. However, I don't think that changes the nature of things. This is a bug fix, not a feature change, and should be treated like other bug fixes. ---------------------------------------- Bug #15928: Constant declaration does not conform to JIS 3017:2013 https://bugs.ruby-lang.org/issues/15928#change-91839 * 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: