Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 527 forks source link

FETCH for tied $" called an odd number of times. #5322

Closed p5pRT closed 11 years ago

p5pRT commented 22 years ago

Migrated from rt.perl.org#8931 (status was 'resolved')

Searchable as RT8931$

p5pRT commented 22 years ago

From @abigail

Created by @abigail

Consider the following program​:

  use strict;   my $i = 0;   sub A​::TIESCALAR {bless [] => 'A'}   sub A​::FETCH {print ++ $i\, "\n"}   my @​a = (""\, ""\, "");

  tie $" => 'A';   "@​a";   __END__

For perl 5.001 up to perl 5.6.0 (and also perl 5.7.0)\, this prints '1'. That is\, if stringification of an array needs the value of $"\, its value is fetched only once for this stringification\, even if $" is required more often.

In 5.6.1 and from 5.7.1 onwards\, this prints '1'\, '2'\, '3'. It looks like the value of $" is fetched each time it appears in the resulting string - but then it's called once too often. If @​a is an empty array\, "@​a" results in the value of $" being fetched!

While the multiple fetching of $" is arguably a change for the better (was this done intentionally\, or was it a side effect?)\, it does contradict the documentation. Perldata says​:

  The following are equivalent​:

  $temp = join($"\, @​ARGV);   system "echo $temp";

  system "echo @​ARGV";

This is no longer the case.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl v5.7.3: Configured by abigail at Fri Apr 12 02:04:48 CEST 2002. Summary of my perl5 (revision 5.0 version 7 subversion 3 patch 15863) configuration: Platform: osname=linux, osvers=2.4.5, archname=i686-linux-64int-ld uname='linux hermione 2.4.5 #6 fri jun 22 01:38:20 pdt 2001 i686 unknown ' config_args='-des -Uversiononly -Dmydomain=.foad.org -Dcf_email=abigail@foad.org -Dperladmin=abigail@foad.org -Doptimize=-g -Dusemorebits -Dusedevel -Dusenm=false -Dprefix=/opt/perl/@15863' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=define usemymalloc=n, bincompat5005=define Compiler: cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/opt/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-g', cppflags='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -I/opt/local/include' ccversion='', gccversion='2.95.3 20010315 (release)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='long double', nvsize=12, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib -L/opt/local/lib' libpth=/usr/local/lib /opt/local/lib /lib /usr/lib libs=-lnsl -lndbm -lgdbm -ldl -lm -lc -lcrypt -lutil perllibs=-lnsl -ldl -lm -lc -lcrypt -lutil libc=/lib/libc-2.2.3.so, so=so, useshrplib=false, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib -L/opt/local/lib' Locally applied patches: DEVEL15844 @INC for perl v5.7.3: /home/abigail/Perl /home/abigail/Sybase /opt/perl/@15863/lib/5.7.3/i686-linux-64int-ld /opt/perl/@15863/lib/5.7.3 /opt/perl/@15863/lib/site_perl/5.7.3/i686-linux-64int-ld /opt/perl/@15863/lib/site_perl/5.7.3 /opt/perl/@15863/lib/site_perl . Environment for perl v5.7.3: HOME=/home/abigail LANG (unset) LANGUAGE (unset) LC_ALL=POSIX LD_LIBRARY_PATH=/home/abigail/Lib:/usr/local/lib:/usr/lib:/lib:/usr/X11R6/lib:/opt/gnome/lib LOGDIR (unset) PATH=/home/abigail/Bin:/opt/perl/bin:/usr/local/bin:/usr/local/X11/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/X11R6/bin:/usr/games:/opt/povray/bin:/opt/gnome/bin:/opt/opera/bin:/usr/share/texmf/bin:/opt/Acrobat4/bin:/opt/java/blackdown/j2sdk1.3.1/bin:/usr/local/games/bin:/opt/gnuplot/bin:/opt/mysql/bin PERL5LIB=/home/abigail/Perl:/home/abigail/Sybase PERLDIR=/opt/perl PERL_BADLANG (unset) SHELL=/usr/bin/bash ```
p5pRT commented 22 years ago

From @andk

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

The responsible patch is 8248.

-- andreas

p5pRT commented 22 years ago

From @jhi

On Fri\, Apr 12\, 2002 at 02​:39​:23PM +0200\, Andreas J. Koenig wrote​:

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

The responsible patch is 8248.

.... which maans that 8264 didn't really fully retract 8248?

-- andreas

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @andk

On Fri\, 12 Apr 2002 16​:05​:55 +0300\, Jarkko Hietaniemi \jhi@​iki\.fi said​:

  > On Fri\, Apr 12\, 2002 at 02​:39​:23PM +0200\, Andreas J. Koenig wrote​:

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

The responsible patch is 8248.

  > ... which maans that 8264 didn't really fully retract 8248?

Correct. After 8248 I see always the same result.

-- andreas

p5pRT commented 22 years ago

From @jhi

On Fri\, Apr 12\, 2002 at 03​:29​:00PM +0200\, Andreas J. Koenig wrote​:

On Fri\, 12 Apr 2002 16​:05​:55 +0300\, Jarkko Hietaniemi \jhi@​iki\.fi said​:

On Fri\, Apr 12\, 2002 at 02​:39​:23PM +0200\, Andreas J. Koenig wrote​:

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

The responsible patch is 8248.

... which maans that 8264 didn't really fully retract 8248?

Correct. After 8248 I see always the same result.

Well\, in any case\, the change for $" in particular was certainly unintentional.

-- andreas

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @abigail

On Fri\, Apr 12\, 2002 at 02​:39​:23PM +0200\, Andreas J. Koenig wrote​:

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

If one has a history of writing Japhs (it's been a while since I wrote the last one)\, one has a bunch of cute test cases.....

It seems that 5.8 is going to break less Japhs than 5.6 did.

The responsible patch is 8248.

But that patch was retracted by patch 8264 according the Changes file.

Abigail

p5pRT commented 22 years ago

From @nwc10

On Fri\, Apr 12\, 2002 at 03​:36​:13PM +0200\, abigail@​foad.org wrote​:

If one has a history of writing Japhs (it's been a while since I wrote the last one)\, one has a bunch of cute test cases.....

It seems that 5.8 is going to break less Japhs than 5.6 did.

So we need t/japh/*.t then? :-)

Nicholas Clark

p5pRT commented 22 years ago

From @jhi

On Fri\, Apr 12\, 2002 at 03​:36​:13PM +0200\, abigail@​foad.org wrote​:

On Fri\, Apr 12\, 2002 at 02​:39​:23PM +0200\, Andreas J. Koenig wrote​:

On 12 Apr 2002 10​:45​:18 -0000\, abigail@​foad.org said​:

Cute testscript.

If one has a history of writing Japhs (it's been a while since I wrote the last one)\, one has a bunch of cute test cases.....

It seems that 5.8 is going to break less Japhs than 5.6 did.

The responsible patch is 8248.

But that patch was retracted by patch 8264 according the Changes file.

Apparently 8264 is lying. My first candidate for the behavioral change goes for this

  for (; items > 0; items--\,mark++) { - sv_catpvn(sv\,delim\,len); + sv_catsv(sv\,del);   sv_catsv(sv\,*mark);   }   }

in the beginning of the 8248​: instead of the PV delim being fetched once and used for all the catenations\, the original delimiter sv is "invoked" anew at every iteration.

Abigail

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @jhi

On Fri\, Apr 12\, 2002 at 02​:41​:22PM +0100\, Nicholas Clark wrote​:

On Fri\, Apr 12\, 2002 at 03​:36​:13PM +0200\, abigail@​foad.org wrote​:

If one has a history of writing Japhs (it's been a while since I wrote the last one)\, one has a bunch of cute test cases.....

It seems that 5.8 is going to break less Japhs than 5.6 did.

So we need t/japh/*.t then? :-)

I think Abigail's arsenal on that would bloat the distribution too much :-)

Nicholas Clark

-- $jhi++; # http​://www.iki.fi/jhi/   # There is this special biologist word we use for 'stable'.   # It is 'dead'. -- Jack Cohen

p5pRT commented 22 years ago

From @abigail

On Fri\, Apr 12\, 2002 at 02​:41​:22PM +0100\, Nicholas Clark wrote​:

On Fri\, Apr 12\, 2002 at 03​:36​:13PM +0200\, abigail@​foad.org wrote​:

If one has a history of writing Japhs (it's been a while since I wrote the last one)\, one has a bunch of cute test cases.....

It seems that 5.8 is going to break less Japhs than 5.6 did.

So we need t/japh/*.t then? :-)

I jokely said something like that on #perl a few hours ago\, and sky said that where I to make such a test suite\, he would put it in.

I'll work on it ASAP. (After I finish complaining to HP about their Perl course material)

Abigail

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

En réponse à abigail@​foad.org​:

On Fri\, Apr 12\, 2002 at 02​:41​:22PM +0100\, Nicholas Clark wrote​:

So we need t/japh/*.t then? :-)

I'd prefer a single t/lib/japh.t (or whatever) that does something like :

require "./test.pl"; # take advantage of runperl() $/ = "#\n"; while (\) {   # check the output\, print (not )?ok... } __DATA__ print "Just another Perl hacker\,\n"; # ....other japhs listed there.

(of course this is in the case where some japhs needs to be run in separate processes / use command-line options. Otherwise an eval() is sufficient.) Beware the execution time also : no need to bloat the test suite -- you know that already\, you run smokes. (or should this be skipped by default ? are obfuscated regression tests that useful ?)

I jokely said something like that on #perl a few hours ago\, and sky said that where I to make such a test suite\, he would put it in.

p5pRT commented 22 years ago

From @abigail

On Fri\, Apr 12\, 2002 at 04​:32​:02PM +0200\, rgarciasuarez@​free.fr wrote​:

(or should this be skipped by default ? are obfuscated regression tests that useful ?)

I think the advantage of "obfuscated" tests is that they combine features in a surprising way.

The test suite probably checks whether $" works as intended\, it also tests whether tieing works correctly. But it's the combination that failed here​: $" being tied to a function with side effects.

Abigail

p5pRT commented 11 years ago

From @Hugmeir

On Fri Apr 12 00​:54​:59 2002\, RT_System wrote​:

On Fri\, Apr 12\, 2002 at 04​:32​:02PM +0200\, rgarciasuarez@​free.fr wrote​:

(or should this be skipped by default ? are obfuscated regression tests that useful ?)

I think the advantage of "obfuscated" tests is that they combine features in a surprising way.

The test suite probably checks whether $" works as intended\, it also tests whether tieing works correctly. But it's the combination that failed here​: $" being tied to a function with side effects.

Abigail

Actually\, it's worse than that​:

my $i = 0; sub A​::TIESCALAR {bless [] => 'A'} sub A​::FETCH {print ++ $i\, "\n"}

tie my $a => 'A'; join $a\, 1..10;

Will call $a's magic for every element. So it's not tie+$"\, but tie+join that are broken. Weird that this ticket ended up creating a whole new section of the test suite\, but didn't get the issue fixed\, eh? In any case\, the attached patch seems to do the trick. It's just one line\, which probably means that it's broken somehow.

--hugmeir

p5pRT commented 11 years ago

From @Hugmeir

0001-Fix-for-perl-8931-call-magic-only-once-for-join-s-fi.patch ```diff From 187f60769a34d9dc1154e76a37c28440ff3d6803 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Fri, 25 May 2012 02:44:20 -0300 Subject: [PATCH] Fix for [perl #8931], call magic only once for join's first arg. --- doop.c | 2 +- t/op/tie.t | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/doop.c b/doop.c index a562536..1bd16b5 100644 --- a/doop.c +++ b/doop.c @@ -720,7 +720,7 @@ Perl_do_join(pTHX_ register SV *sv, SV *delim, register SV **mark, register SV * if (delimlen) { for (; items > 0; items--,mark++) { - sv_catsv(sv,delim); + sv_catsv_nomg(sv,delim); sv_catsv(sv,*mark); } } diff --git a/t/op/tie.t b/t/op/tie.t index 9301bb3..fcbf7a5 100644 --- a/t/op/tie.t +++ b/t/op/tie.t @@ -1259,3 +1259,21 @@ $h{i}{j} = 'k'; print $h{i}{j}, "\n"; EXPECT k +######## + +# [perl #8931] FETCH for tied $" called an odd number of times. +use strict; +my $i = 0; +sub A::TIESCALAR {bless [] => 'A'} +sub A::FETCH {print ++ $i, "\n"} +my @a = ("", "", ""); + +tie $" => 'A'; +"@a"; + +$i = 0; +tie my $a => 'A'; +join $a, 1..10; +EXPECT +1 +1 -- 1.7.9.5 ```
p5pRT commented 11 years ago

From @cpansprout

On Thu May 24 22​:51​:45 2012\, Hugmeir wrote​:

In any case\, the attached patch seems to do the trick. It's just one line\, which probably means that it's broken somehow.

Not broken at all. :-)

Thank you. Applied as 760209f.

--

Father Chrysostomos

p5pRT commented 11 years ago

@cpansprout - Status changed from 'open' to 'resolved'