Project

General

Profile

Actions

Backport #8872

closed

Case statements do not honor a refinement of the '===' method

Added by jconley88 (Jon Conley) over 10 years ago. Updated about 10 years ago.


Description

=begin
Below, I've redefined the ((|===|)) method of symbol to always return true. In ((|RefineTest#uses_refinement|)), I call ((|===|)) directly and the refined method is called. In ((|RefineTest#does_not_use_refinement|)), the ((|===|)) method is called indirectly through a case statement. If the refined ((|===|)) method was called, the result should be (('The refinement was used')), but this code currently returns (('The refinement was not used')).

module RefineSymbol
refine Symbol do
def ===(other)
true
end
end
end

using RefineSymbol

class RefineTest
def uses_refinement
:a === :b
end

def does_not_use_refinement
case :a
when :b
'The refinement was used'
else
'The refinement was not used'
end
end
end

rt = RefineTest.new
rt.uses_refinement # => true
rt.does_not_use_refinement # => expected 'The refinement was used' but got 'The refinement was not used'
=end


Related issues 2 (0 open2 closed)

Related to Backport200 - Backport #9175: Backport r43913Closednagachika (Tomoyuki Chikanaga)11/29/2013Actions
Has duplicate Ruby master - Bug #9150: Segfault in case statement execution, possibly related to refinementsClosednagachika (Tomoyuki Chikanaga)11/25/2013Actions
Actions #1

Updated by Anonymous over 10 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r42869.
Jon, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • vm_eval.c (vm_call0): fix prototype, the id parameter should be of
    type ID, not VALUE

  • vm_insnhelper.c (check_match): the rb_funcall family of functions
    does not care about refinements. We need to use
    rb_method_entry_with_refinements instead to call === with
    refinements. Thanks to Jon Conley for reporting this bug.
    [ruby-core:57051] [Bug #8872]

  • test/ruby/test_refinement.rb: add test

Updated by Anonymous over 10 years ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: DONTNEED, 2.0.0: REQUIRED
Actions #3

Updated by nagachika (Tomoyuki Chikanaga) over 10 years ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby master to Backport200
  • Status changed from Closed to Assigned
  • Assignee set to nagachika (Tomoyuki Chikanaga)
Actions #4

Updated by nagachika (Tomoyuki Chikanaga) over 10 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r42923.
Jon, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


merge revision(s) 42869: [Backport #8872]

* vm_eval.c (vm_call0): fix prototype, the id parameter should be of
  type ID, not VALUE

* vm_insnhelper.c (check_match): the rb_funcall family of functions
  does not care about refinements. We need to use
  rb_method_entry_with_refinements instead to call === with
  refinements. Thanks to Jon Conley for reporting this bug.
  [ruby-core:57051] [Bug #8872]

* test/ruby/test_refinement.rb: add test

Updated by pixeltrix (Andrew White) over 10 years ago

This has caused a regression in Rails where === isn't being sent via method_missing:

https://github.com/rails/rails/pull/13055

This results in a segmentation fault.

Updated by sorah (Sorah Fukumori) over 10 years ago

@pixeltrix (Andrew White) Fixed at r43913, Thank you for your reporting

Updated by shugo (Shugo Maeda) over 10 years ago

  • Status changed from Closed to Assigned
  • Assignee changed from nagachika (Tomoyuki Chikanaga) to matz (Yukihiro Matsumoto)

As I stated in #9150, I doubt this feature conforms to the design policy of refinements.
r42869 should be reverted, shouldn't it, Matz?

Updated by shugo (Shugo Maeda) over 10 years ago

I strongly request committers not to change the behavior of Refinements without Matz's permission, except when there's an obvious bug such as SEGV in Refinements, because the current feature set of Refinements are restricted to keep compatibility with other implementations such as JRuby.

In general, refinements should be activated only in a certain lexical scope.

Actions #9

Updated by sorah (Sorah Fukumori) over 10 years ago

shugo (Shugo Maeda) wrote:

I strongly request committers not to change the behavior of Refinements without Matz's permission, except when there's an obvious bug such as SEGV in Refinements, because the current feature set of Refinements are restricted to keep compatibility with other implementations such as JRuby.

In general, refinements should be activated only in a certain lexical scope.

Agreed.

Note that my commit (r43913) just fixes bug in the current behavior, please revert it too with r42839, if we revert r42839.

Actions #10

Updated by matz (Yukihiro Matsumoto) over 10 years ago

The basic principle of refinements is that refinement do work in local scope.
So methods called from the scope will be refined but method called from methods called will not.

And very importantly, "===" called from case statement is controversial.
It can be viewed as a method called from case statement, or case statement itself is just a syntax sugar wrapping "===" calls. From the former point of view, refine should not affect "===", but from the latter POV, refinement should change the behavior.

Right now, we took the former POV, and OP expected the latter.
So I would like to hear from others.

Matz.

Updated by shugo (Shugo Maeda) over 10 years ago

matz (Yukihiro Matsumoto) wrote:

And very importantly, "===" called from case statement is controversial.
It can be viewed as a method called from case statement, or case statement itself is just a syntax sugar wrapping "===" calls. From the former point of view, refine should not affect "===", but from the latter POV, refinement should change the behavior.

Ruby has other features which call methods implicitly. For example, Range#to_a is called by the following code:

a = *[1..10]

Should refinements be honored in all such features?

Updated by shugo (Shugo Maeda) over 10 years ago

I've found that for expressions honor refinements.

module RefineEach
refine Array do
def each
yield "refined"
end
end
end

using RefineEach

for i in [1, 2, 3]
p i
end

for expressions seem to be more closely related to method calls, but I'm not sure how refinements should be handled in these cases....

Updated by Eregon (Benoit Daloze) over 10 years ago

shugo (Shugo Maeda) wrote:

a = *[1..10]

You probably meant

a = [*1..10]

Updated by shugo (Shugo Maeda) over 10 years ago

Eregon (Benoit Daloze) wrote:

a = *[1..10]

You probably meant

a = [*1..10]

Ah, I meant:

a = *1..10

Thanks anyway.

Updated by shugo (Shugo Maeda) over 10 years ago

Could anyone tell me use cases of this feature?

I think refinements are for object-oriented ways using dynamic dispatch, but case expressions are not for such object-oriented ways.

Updated by jballanc (Joshua Ballanco) over 10 years ago

To play devil's advocate, I think the opposing question to be asked is "how would you alter the behavior of case evaluation using refinements"?

It may be, as shugo states, that refinements are not meant to be used for altering the behavior of expressions. In this case, the answer to the question I posed is, simply, "you don't."

That said, I think it is fairly well understood in the Ruby community that "===" is related to case. I've even heard the operator described as "case equality". Playing devil's advocate again, what is the use case for refining "===" if it doesn't affect the evaluation of a case...when clause?

Updated by shugo (Shugo Maeda) over 10 years ago

jballanc (Joshua Ballanco) wrote:

To play devil's advocate, I think the opposing question to be asked is "how would you alter the behavior of case evaluation using refinements"?

It may be, as shugo states, that refinements are not meant to be used for altering the behavior of expressions. In this case, the answer to the question I posed is, simply, "you don't."

That said, I think it is fairly well understood in the Ruby community that "===" is related to case. I've even heard the operator described as "case equality". Playing devil's advocate again, what is the use case for refining "===" if it doesn't affect the evaluation of a case...when clause?

I don't come up with any use case for refining "===" regardless of whether it affects the evaluation of a case expression or not.
I'm neutral about this issue, and really want to know use cases.

Updated by nagachika (Tomoyuki Chikanaga) over 10 years ago

hi,

Is there any progress? Does anyone has usecase of refinements & case statement?

Updated by matz (Yukihiro Matsumoto) over 10 years ago

After consideration, I think for and case statement should honor refinement since they are fundamentally syntax sugar using each/=== methods.

Matz.

Updated by nagachika (Tomoyuki Chikanaga) over 10 years ago

How about 2.0.0? Can I change the behavior at 2.0.0 to same with 2.1?
If so I'll backport r43913 to fix SEGV.

Actions #21

Updated by matz (Yukihiro Matsumoto) about 10 years ago

  • ruby -v set to 2.0.0

I don't think changing the behavior in the middle of a release is a good idea.
So I don't encourage backporting. Thanks.

Matz.

Actions #22

Updated by matz (Yukihiro Matsumoto) about 10 years ago

  • ruby -v changed from 2.0.0 to -

Issue #8872 has been updated by Yukihiro Matsumoto.

ruby -v set to 2.0.0

I don't think changing the behavior in the middle of a release is a good idea.
So I don't encourage backporting. Thanks.

Matz.


Backport #8872: Case statements do not honor a refinement of the '===' method
https://bugs.ruby-lang.org/issues/8872#change-44355

  • Author: Jon Conley
  • Status: Assigned
  • Priority: Normal
  • Assignee: Yukihiro Matsumoto
  • Category:
  • Target version:
  • ruby -v: 2.0.0

=begin
Below, I've redefined the ((|===|)) method of symbol to always return true. In ((|RefineTest#uses_refinement|)), I call ((|===|)) directly and the refined method is called. In ((|RefineTest#does_not_use_refinement|)), the ((|===|)) method is called indirectly through a case statement. If the refined ((|===|)) method was called, the result should be (('The refinement was used')), but this code currently returns (('The refinement was not used')).

module RefineSymbol
refine Symbol do
def ===(other)
true
end
end
end

using RefineSymbol

class RefineTest
def uses_refinement
:a === :b
end

def does_not_use_refinement
  case :a
  when :b
    'The refinement was used'
  else
    'The refinement was not used'
  end
end

end

rt = RefineTest.new
rt.uses_refinement # => true
rt.does_not_use_refinement # => expected 'The refinement was used' but got 'The refinement was not used'
=end

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

Updated by nagachika (Tomoyuki Chikanaga) about 10 years ago

  • Assignee changed from matz (Yukihiro Matsumoto) to nagachika (Tomoyuki Chikanaga)

Hi,

I've told matz that this change introduced by r42923 was already released at 2.0.0p353 and matz decided to not to revert the changeset. (see https://twitter.com/yukihiro_matz/status/423500823788662784 )

I will backport r43913 to fix SEGV introduced at r42923.

Thanks.

Updated by nagachika (Tomoyuki Chikanaga) about 10 years ago

  • Status changed from Assigned to Closed

Applied in changeset r44695.


merge revision(s) 43913: [Backport #8872] [Backport #9175]

* vm_insnhelper.c (check_match): Fix SEGV with VM_CHECKMATCH_TYPE_CASE
  and class of `pattern` has `method_missing`
  [Bug #8882] [ruby-core:58606]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0