Project

General

Profile

Actions

Feature #15751

closed

Add FrozenError#receiver

Added by jeremyevans0 (Jeremy Evans) over 5 years ago. Updated over 5 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:92165]

Description

Similar to NameError#receiver, this returns the object on which
the modification was attempted. This is useful as it can pinpoint
exactly what is frozen, as in many cases when a FrozenError is
raised, you cannot determine from the context which object is
frozen that you expect not to be.

I'm not sure that using name_err_receiver as the C function for
FrozenError#receiver is acceptable, but it doesn't appear to cause
problems.

Users of the current rb_error_frozen C function will have to switch
to using rb_error_frozen_object or the new rb_raise_frozen_error
in order to set the receiver of the FrozenError.

Patch to implement this feature is attached.


Files

Add-FrozenError-receiver.patch (6.48 KB) Add-FrozenError-receiver.patch jeremyevans0 (Jeremy Evans), 04/06/2019 07:05 AM
Add-FrozenError-receiver-v2.patch (6.66 KB) Add-FrozenError-receiver-v2.patch jeremyevans0 (Jeremy Evans), 04/06/2019 06:00 PM
Add-FrozenError-receiver-v3.patch (6.49 KB) Add-FrozenError-receiver-v3.patch jeremyevans0 (Jeremy Evans), 04/07/2019 03:36 AM

Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #16419: FrozenError.new ignores receiver:Closedmatz (Yukihiro Matsumoto)Actions

Updated by nobu (Nobuyoshi Nakada) over 5 years ago

  • Description updated (diff)

Seems reasonable, but shouldn't FrozenError#initialize use a keyword argument as well as NameError#initialize?

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

nobu (Nobuyoshi Nakada) wrote:

Seems reasonable, but shouldn't FrozenError#initialize use a keyword argument as well as NameError#initialize?

NameError#initialize uses an additional positional argument for the name argument, and I figured that was easier as it is unlikely FrozenError#initialize would need to support additional arguments. However, I don't have strong feelings about it, so attached is a patch that switches to using a keyword argument.

Updated by Eregon (Benoit Daloze) over 5 years ago

NameError#initialize uses a keyword argument for receiver, so I it makes some sense to be consistent.

OTOH, keyword arguments often imply extra processing, especially if the caller is not Ruby code, so I actually find the positional argument version simpler.
And indeed, I don't think extra arguments will be needed for FrozenError#initialize.

In the patch in rb_raise_frozen_error, exc is used for both the message and the exception object, could you use different variables?
I find it confusing to use the same and re-assign it when there is no need.

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

Eregon (Benoit Daloze) wrote:

In the patch in rb_raise_frozen_error, exc is used for both the message and the exception object, could you use different variables?
I find it confusing to use the same and re-assign it when there is no need.

Sure, I can make that change. Can you and nobu come to an agreement on whether to use the positional argument or the keyword argument so I know which patch to update?

Updated by nobu (Nobuyoshi Nakada) over 5 years ago

I haven't look C-API well, the additional argument would be ok.
rb_raise_frozen_error should be rb_frozen_error_raise like rb_name_err_raise?

As for the test, assert_same would be better for the equality of the receiver.

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

nobu (Nobuyoshi Nakada) wrote:

I haven't look C-API well, the additional argument would be ok.
rb_raise_frozen_error should be rb_frozen_error_raise like rb_name_err_raise?

As for the test, assert_same would be better for the equality of the receiver.

OK, v3 of the patch is attached, based off the initial patch with the positional argument, with the function renamed to rb_frozen_error_raise and using assert_same in the tests.

Eregon (Benoit Daloze) wrote:

In the patch in rb_raise_frozen_error, exc is used for both the message and the exception object, could you use different variables?
I find it confusing to use the same and re-assign it when there is no need.

v3 patch includes this change as well.

Updated by Eregon (Benoit Daloze) over 5 years ago

The patch looks good to me.
I will let @nobu (Nobuyoshi Nakada) commit it.

A nice extra would be specs in spec/ruby/core/exception/frozen_error_spec.rb (similar to spec/ruby/core/exception/name_error_spec.rb) but this is not a requirement.

Actions #8

Updated by jeremyevans (Jeremy Evans) over 5 years ago

  • Status changed from Open to Closed

Applied in changeset git|39eadca76b48fc7841da688f6745e40897ec37ff.


Add FrozenError#receiver

Similar to NameError#receiver, this returns the object on which
the modification was attempted. This is useful as it can pinpoint
exactly what is frozen. In many cases when a FrozenError is
raised, you cannot determine from the context which object is
frozen that you attempted to modify.

Users of the current rb_error_frozen C function will have to switch
to using rb_error_frozen_object or the new rb_frozen_error_raise
in order to set the receiver of the FrozenError.

To allow the receiver to be set from Ruby, support an optional
second argument to FrozenError#initialize.

Implements [Feature #15751]

Actions #9

Updated by znz (Kazuhiro NISHIYAMA) almost 5 years ago

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0