https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112019-04-06T13:44:54ZRuby Issue Tracking SystemRuby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774932019-04-06T13:44:54Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/77493/diff?detail_id=51562">diff</a>)</li></ul><p>Seems reasonable, but shouldn't <code>FrozenError#initialize</code> use a keyword argument as well as <code>NameError#initialize</code>?</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774942019-04-06T18:04:38Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul><li><strong>File</strong> <a href="/attachments/7738">Add-FrozenError-receiver-v2.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/7738/Add-FrozenError-receiver-v2.patch">Add-FrozenError-receiver-v2.patch</a> added</li></ul><p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>Seems reasonable, but shouldn't <code>FrozenError#initialize</code> use a keyword argument as well as <code>NameError#initialize</code>?</p>
</blockquote>
<p><code>NameError#initialize</code> uses an additional positional argument for the <code>name</code> argument, and I figured that was easier as it is unlikely <code>FrozenError#initialize</code> 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.</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774952019-04-06T19:14:07ZEregon (Benoit Daloze)
<ul></ul><p><code>NameError#initialize</code> uses a keyword argument for <code>receiver</code>, so I it makes some sense to be consistent.</p>
<p>OTOH, keyword arguments often imply extra processing, especially if the caller is not Ruby code, so I actually find the positional argument version simpler.<br>
And indeed, I don't think extra arguments will be needed for <code>FrozenError#initialize</code>.</p>
<p>In the patch in <code>rb_raise_frozen_error</code>, <code>exc</code> is used for both the message and the exception object, could you use different variables?<br>
I find it confusing to use the same and re-assign it when there is no need.</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774962019-04-06T22:09:24Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul></ul><p>Eregon (Benoit Daloze) wrote:</p>
<blockquote>
<p>In the patch in <code>rb_raise_frozen_error</code>, <code>exc</code> is used for both the message and the exception object, could you use different variables?<br>
I find it confusing to use the same and re-assign it when there is no need.</p>
</blockquote>
<p>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?</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774972019-04-07T03:04:59Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul></ul><p>I haven't look C-API well, the additional argument would be ok.<br>
<code>rb_raise_frozen_error</code> should be <code>rb_frozen_error_raise</code> like <code>rb_name_err_raise</code>?</p>
<p>As for the test, <code>assert_same</code> would be better for the equality of the receiver.</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=774982019-04-07T03:40:23Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul><li><strong>File</strong> <a href="/attachments/7739">Add-FrozenError-receiver-v3.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/7739/Add-FrozenError-receiver-v3.patch">Add-FrozenError-receiver-v3.patch</a> added</li></ul><p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>I haven't look C-API well, the additional argument would be ok.<br>
<code>rb_raise_frozen_error</code> should be <code>rb_frozen_error_raise</code> like <code>rb_name_err_raise</code>?</p>
<p>As for the test, <code>assert_same</code> would be better for the equality of the receiver.</p>
</blockquote>
<p>OK, v3 of the patch is attached, based off the initial patch with the positional argument, with the function renamed to <code>rb_frozen_error_raise</code> and using <code>assert_same</code> in the tests.</p>
<p>Eregon (Benoit Daloze) wrote:</p>
<blockquote>
<p>In the patch in <code>rb_raise_frozen_error</code>, <code>exc</code> is used for both the message and the exception object, could you use different variables?<br>
I find it confusing to use the same and re-assign it when there is no need.</p>
</blockquote>
<p>v3 patch includes this change as well.</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=775092019-04-07T17:07:47ZEregon (Benoit Daloze)
<ul></ul><p>The patch looks good to me.<br>
I will let <a class="user active user-mention" href="https://redmine.ruby-lang.org/users/4">@nobu (Nobuyoshi Nakada)</a> commit it.</p>
<p>A nice extra would be specs in <code>spec/ruby/core/exception/frozen_error_spec.rb</code> (similar to <code>spec/ruby/core/exception/name_error_spec.rb</code>) but this is not a requirement.</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=782322019-05-26T18:10:19Zjeremyevans (Jeremy Evans)code@jeremyevans.net
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li></ul><p>Applied in changeset <a class="changeset" title="Add FrozenError#receiver Similar to NameError#receiver, this returns the object on which the mod..." href="https://redmine.ruby-lang.org/projects/ruby-master/repository/git/revisions/39eadca76b48fc7841da688f6745e40897ec37ff">git|39eadca76b48fc7841da688f6745e40897ec37ff</a>.</p>
<hr>
<p>Add FrozenError#receiver</p>
<p>Similar to NameError#receiver, this returns the object on which<br>
the modification was attempted. This is useful as it can pinpoint<br>
exactly what is frozen. In many cases when a FrozenError is<br>
raised, you cannot determine from the context which object is<br>
frozen that you attempted to modify.</p>
<p>Users of the current rb_error_frozen C function will have to switch<br>
to using rb_error_frozen_object or the new rb_frozen_error_raise<br>
in order to set the receiver of the FrozenError.</p>
<p>To allow the receiver to be set from Ruby, support an optional<br>
second argument to FrozenError#initialize.</p>
<p>Implements [Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Add FrozenError#receiver (Closed)" href="https://redmine.ruby-lang.org/issues/15751">#15751</a>]</p> Ruby master - Feature #15751: Add FrozenError#receiverhttps://redmine.ruby-lang.org/issues/15751?journal_id=831192019-12-14T07:29:57Zznz (Kazuhiro NISHIYAMA)
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-5 priority-4 priority-default closed" href="/issues/16419">Feature #16419</a>: FrozenError.new ignores receiver:</i> added</li></ul>