New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FETCH called twice #10521
Comments
From @AbigailCreated by @AbigailA couple of my CPAN modules broke recently, due to FETCH, under certain This is a reduced test case: use 5.010; my $count; sub TIESCALAR {bless []} tie my $foo => "main"; {no warnings 'void'; $foo eq 'foo';} say $count; __END__ This should print '1'. However, with blead, it prints 2. git bisect tells commit 6f1401d is the commit 6f1401d make overload respect get magic In most places, ops checked their args for overload *before* doing This patch does the following: Makes sure get magic is called first. Makes sure that FETCH is not called multiple times. Much of this I'll write a couple of test cases for this and commit them. Abigail Perl Info
|
From @AbigailOn Tue, Jul 27, 2010 at 11:42:19PM +0200, Abigail wrote:
I wrote some tests, and I found 13 cases in blead where FETCH is called $tied lt 1; !$tied $tied || 1; m/$tied/ 5.12.1 is slightly better, but there are still 9 cases where FETCH is $tied || 1; abs $tied; -t $tied; m/$tied/; So, four cases are fixed in blead, five are still present, and eight new 5.8.8 calls FETCH just once for m/$tied/, s/$tied// and -t $tied, *three* New tests are in t/op/tie_fetch_count.t. Abigail |
From @cpansprout• The first patch fixes string comparison operators. It changes sv_eq, sv_cmp, sv_cmp_locale and sv_collxfrm to _flags forms, with macros under the old names for sv_eq and sv_collxfrm, but functions for sv_cmp* since pp_sort.c needs them. • The second patch fixes ! by doing the same thing to sv_2bool and adding SvTRUE_nomg. It also corrects the docs that state incorrectly that SvTRUE does not handle magic. • The third patch fixes m and s. It modifies pp_regcomp to avoid extra magic. It also corrects a bug in sv_catsv_flags, which would still call mg_get(ssv) even without the SV_GMAGIC flag set. • The fourth patch fixes y. (This has caused double magick at least as far back as 5.6.2.) • The || case is not a bug, as there are two separate operators operating on it in the test script. In $dummy = $x || $y The || does mg_get($x). If it’s true it returns it and the = does mg_get($x). If $x is false, then $y is returned, so magic is called once on each of $x and $y. Similarly, && will seemingly call mg_get($x) twice if $x is false. If you just write: $x || $y then magic is only called once on $x. The fifth patch attached hereto corrects the test. • The sixth attachment is not a patch, but a text file of entries to be copied and pasted into perldelta, patches for which go stale too quickly. Let’s see whether I remember the attachments.... |
From @cpansprout[1. string comparison](https://rt-archive.perl.org/perl5/Ticket/Attachment/720604/345754/1. string comparison) |
From @cpansprout[3. m and s](https://rt-archive.perl.org/perl5/Ticket/Attachment/720604/345758/3. m and s) |
From @cpansprout================================ =head3 L<perlapi> The documentation for the C<SvTRUE> macro was simply wrong in stating that ======= =item * The new F<t/op/tie_fetch_count.t> script tests that operators only call ================ =item New C<*_flags> functions for string comparison The C<sv_cmp_flags>, C<sv_cmp_locale_flags>, C<sv_eq_flags> and The C<sv_cmp>, C<sv_cmp_locale>, C<sv_eq> and C<sv_collxfrm> functions have =item New C<sv_2bool_flags> function This is like C<sv_2bool>, but it lets the calling code decide whether =item New C<SvTRUE_nomg> macro This is just like C<SvTRUE>, except that it does not process magic. It uses =item C<sv_catsv_flags> fix C<sv_catsv_flags> no longer calls C<mg_get> on its second argument (the ================== =item * The C<y///> or C<tr///> operator now calls get-magic (e.g., the C<FETCH> =item * =for comment String comparison (C<eq>, C<ne>, C<lt>, C<gt>, C<le>, C<ge> and =item * =for comment When a tied (or other magic) variable is used as, or in, a regular |
The RT System itself - Status changed from 'new' to 'open' |
From @AbigailOn Sun, Aug 01, 2010 at 12:14:52PM -0700, Father Chrysostomos wrote: Thank you for your patches.
I don't understand why it's not a bug. If I do $dummy = f () || g (); people would consider it to be a bug if f () were to be called twice. Why
Abigail |
From @cpansproutOn Aug 19, 2010, at 8:09 AM, Abigail wrote:
(Except that (tied $x)->FETCH doesn’t allow $_[0] assignment to change what the variable is tied too. :-) Is it really rvalue context? Consider the following examples: $dummy = \($x || $y); Can you come up with a way to unify the concepts such that your test would work and my two examples would continue to work (oh, and ${\($a||$b)} = $c as well)? In the mean time, is there any chance the first four patches could be applied?
|
From @iabynOn Sun, Aug 22, 2010 at 12:16:19PM -0700, Father Chrysostomos wrote:
Could you re-submit the patches, b) Could the patches be attached to the email in a format that 'git am' Thanks. -- |
From @cpansproutOn Aug 24, 2010, at 2:38 PM, Dave Mitchell wrote:
If we do not maintain binary compatibility in blead, then why are we maintaining binary compatibility in blead? Or are these actually for code using the Perl_ forms?
Here they are. Please note that patch number 5 (||) is still controversial, as I’m waiting for an answer from Abigail. BTW, the same thing happens with *{}, which returns its operand if it is already a glob. |
From @cpansprout[1a. string comparison](https://rt-archive.perl.org/perl5/Ticket/Attachment/729764/350498/1a. string comparison) |
From @cpansprout[3a. m and s](https://rt-archive.perl.org/perl5/Ticket/Attachment/729764/350502/3a. m and s) |
From @cpansprout[5a. ||](https://rt-archive.perl.org/perl5/Ticket/Attachment/729764/350506/5a. ||) |
From @cpansprout[7. mathoms](https://rt-archive.perl.org/perl5/Ticket/Attachment/729764/350508/7. mathoms) |
From @iabynOn Sun, Aug 29, 2010 at 12:31:15PM -0700, Father Chrysostomos wrote:
So that patches pulled into maint won't break binary compatibility. -- |
From @nwc10On Mon, Aug 30, 2010 at 02:43:23PM +0100, Dave Mitchell wrote:
If it's possible to do something in a binary compatible way, then the This leaves the option open of merging that change to maint, without needing Nicholas Clark |
From @cpansproutOn Thu Jul 29 14:15:27 2010, abigail@abigail.be wrote:
They should work now. :-) On Sun Aug 29 12:31:49 2010, sprout wrote:
All applied, except for the || patch, as: |
From @cpansproutOn Sun Aug 22 12:16:46 2010, sprout wrote:
That was not a rhetorical question. Have you had a chance to look into |
From @iabynI'm closing this ticket. I think sprout's original diagnosis that To counter the "f() only called once" argument on $a = f() || g(), |
@iabyn - Status changed from 'open' to 'resolved' |
From @cpansproutOn Fri Feb 25 08:49:47 2011, davem wrote:
In that case, I’ll apply the last patch.... Now applied as 1c3caf3. |
Migrated from rt.perl.org#76814 (status was 'resolved')
Searchable as RT76814$
The text was updated successfully, but these errors were encountered: