Skip Menu |
Report information
Id: 132783
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: slaven [at] rezic.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: 5.27.9



CC: srezic [...] cpan.org
To: perlbug [...] perl.org
From: slaven [...] rezic.de
Date: Mon, 29 Jan 2018 21:24:40 +0100
Subject: Blead Breaks CPAN: ADAMK/SQL-String-0.02.tar.gz
Download (untitled) / with headers
text/plain 3.3k
This is a bug report for perl from slaven@rezic.de, generated with the help of perlbug 1.41 running under perl 5.27.8. ----------------------------------------------------------------- The test suite of SQL-String-0.02 started to fail with 5.27.7 (there's no bisect result yet): ... # Looks like you planned 78 tests but ran 79. t/02_main.t ..... Dubious, test returned 255 (wstat 65280, 0xff00) All 78 subtests passed ... On first glance it seems that we have now a warning more than with 5.27.6. ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.27.8: Configured by eserte at Sat Jan 20 09:22:10 CET 2018. Summary of my perl5 (revision 5 version 27 subversion 8) configuration: Platform: osname=linux osvers=3.16.0-4-amd64 archname=x86_64-linux uname='linux cabulja 3.16.0-4-amd64 #1 smp debian 3.16.51-3 (2017-12-13) x86_64 gnulinux ' config_args='-ds -e -Dprefix=/opt/perl-5.27.8 -Dusedevel -Dusemallocwrap=no -Dcf_email=srezic@cpan.org' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='4.9.2' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.19.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong' --- @INC for perl 5.27.8: /opt/perl-5.27.8/lib/site_perl/5.27.8/x86_64-linux /opt/perl-5.27.8/lib/site_perl/5.27.8 /opt/perl-5.27.8/lib/5.27.8/x86_64-linux /opt/perl-5.27.8/lib/5.27.8 --- Environment for perl 5.27.8: HOME=/home/eserte LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/eserte/bin/linux-gnu:/home/eserte/bin/sh:/home/eserte/bin:/home/eserte/bin/pistachio-perl/bin:/usr/games:/home/eserte/devel PERLDOC=-MPod::Perldoc::ToTextOverstrike PERL_BADLANG (unset) SHELL=/bin/zsh
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Mon, 29 Jan 2018 20:25:24 GMT, slaven@rezic.de wrote: Show quoted text
> > This is a bug report for perl from slaven@rezic.de, > generated with the help of perlbug 1.41 running under perl 5.27.8. > > > ----------------------------------------------------------------- > The test suite of SQL-String-0.02 started to fail with 5.27.7 > (there's no bisect result yet): > > ... > # Looks like you planned 78 tests but ran 79. > t/02_main.t ..... > Dubious, test returned 255 (wstat 65280, 0xff00) > All 78 subtests passed > ... > > On first glance it seems that we have now a warning more than > with 5.27.6. >
The additional warning is caught in a test related to overloading of the .= operator (approx test #47 in original). ##### $ cat 132783-Sql-String-overload-dotequals.t use strict; BEGIN { $^W = 1; } use Test::More qw(no_plan); # tests => 78; use SQL::String (); my $SQL = SQL::String->new('foo = ?', 2); { local $SIG{__WARN__} = sub { ok( 1, 'Caught warning during undef concat' ) }; $SQL .= undef; } isa_ok( $SQL, 'SQL::String' ); ##### Results under perl-5.26.0's prove: ##### ok 1 - Caught warning during undef concat ok 2 - An object of class 'SQL::String' isa 'SQL::String' 1..2 ok All tests successful. Files=1, Tests=2, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.05 cusr 0.00 csys = 0.07 CPU) Result: PASS ##### Results under blead's prove: ##### ok 1 - Caught warning during undef concat ok 2 - Caught warning during undef concat ok 3 - An object of class 'SQL::String' isa 'SQL::String' 1..3 ok All tests successful. Files=1, Tests=3, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.03 cusr 0.00 csys = 0.05 CPU) Result: PASS ##### -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 665b
On Mon, 29 Jan 2018 12:25:24 -0800, slaven@rezic.de wrote: Show quoted text
> The test suite of SQL-String-0.02 started to fail with 5.27.7 > (there's no bisect result yet):
Bisect points to d7e75038064881b413f76de9315a5acfb21472f0 is the first bad commit commit d7e75038064881b413f76de9315a5acfb21472f0 Author: David Mitchell <davem@iabyn.com> Date: Tue Nov 28 09:08:09 2017 +0000 $overloaded .= $x: don't stringify $x RT #132385 This is a variant of the ($ref . $overloaded) bug which was fixed with v5.27.5-195-gb3ab0375cb. Basically, when the overloaded concat method is called, it should pass $x as-is, rather than as "$x". This fixes PDL-2.018
To: Sergey Aleynikov via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #132783] Blead Breaks CPAN: ADAMK/SQL-String-0.02.tar.gz
Date: Tue, 30 Jan 2018 15:38:27 +0000
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 1.4k
On Tue, Jan 30, 2018 at 01:03:23AM -0800, Sergey Aleynikov via RT wrote: Show quoted text
> On Mon, 29 Jan 2018 12:25:24 -0800, slaven@rezic.de wrote:
> > The test suite of SQL-String-0.02 started to fail with 5.27.7 > > (there's no bisect result yet):
> > Bisect points to > > d7e75038064881b413f76de9315a5acfb21472f0 is the first bad commit > commit d7e75038064881b413f76de9315a5acfb21472f0 > Author: David Mitchell <davem@iabyn.com> > Date: Tue Nov 28 09:08:09 2017 +0000 > > $overloaded .= $x: don't stringify $x > > RT #132385 > > This is a variant of the ($ref . $overloaded) bug which was fixed with > v5.27.5-195-gb3ab0375cb. > > Basically, when the overloaded concat method is called, it should pass > $x as-is, rather than as "$x". This fixes PDL-2.018
The test case reduces to use warnings; package Foo { use overload '.=' => sub { return "foo"; }; } my $s = bless [], 'Foo'; my $x; $s .= $x; which used to be silent, but now emits: Use of uninitialized value $x in concatenation (.) or string I'll look into this further when I can find some time. -- No man treats a motor car as foolishly as he treats another human being. When the car will not go, he does not attribute its annoying behaviour to sin, he does not say, You are a wicked motorcar, and I shall not give you any more petrol until you go. He attempts to find out what is wrong and set it right. -- Bertrand Russell, Has Religion Made Useful Contributions to Civilization?
Date: Wed, 31 Jan 2018 12:01:33 +0000
Subject: Re: [perl #132783] Blead Breaks CPAN: ADAMK/SQL-String-0.02.tar.gz
To: <perl5-porters [...] perl.org>
From: Smylers <smylers [...] stripey.com>
Download (untitled) / with headers
text/plain 960b
Dave Mitchell writes: Show quoted text
> The test case reduces to > > use warnings; > package Foo { use overload '.=' => sub { return "foo"; }; } > my $s = bless [], 'Foo'; > my $x; > $s .= $x; > > which used to be silent, but now emits: > > Use of uninitialized value $x in concatenation (.) or string > > I'll look into this further when I can find some time.
Why is it wrong for that to warn? $x is undef, and is being supplied to the concatenation operator. Admittedly an overloaded operator that in this case doesn't care that it's undef, but why should Perl start presuming that undef-s are acceptable simply because an operator has been overloaded? I couldn't see anything in overload or in perldiag mentioning that this warning doesn't apply to an overloaded operation. Many uses of overloading are intended to be largely transparent to the user, so they may be surprised to not get a warning that they requested. Smylers
To: perl5-porters [...] perl.org
Subject: Re: [perl #132783] Blead Breaks CPAN: ADAMK/SQL-String-0.02.tar.gz
From: Zefram <zefram [...] fysh.org>
Date: Wed, 31 Jan 2018 13:10:33 +0000
Download (untitled) / with headers
text/plain 653b
Smylers wrote: Show quoted text
>Why is it wrong for that to warn?
The question is whether .= stringifies its rhs in this case. If it stringifies, passing an actual string to the lhs overload, then it is correct to gripe about undef. If it does not stringify, and so passes the rhs value to the lhs overload as-is, then it is up to the overload method to determine what operand values are valid, and .= has no business opining on the matter. In fact both 5.26.0 and 5.27.8 are agreed: .= does not stringify the rhs in this case. They do not call stringification overloads, and do not flatten references. They pass the rhs value to the lhs overload as-is. -zefram
CC: perl5-porters [...] perl.org
Date: Tue, 20 Feb 2018 09:20:03 +0000
Subject: Re: [perl #132783] Blead Breaks CPAN: ADAMK/SQL-String-0.02.tar.gz
To: Zefram <zefram [...] fysh.org>
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 3.6k
On Wed, Jan 31, 2018 at 01:10:33PM +0000, Zefram wrote: Show quoted text
> Smylers wrote:
> >Why is it wrong for that to warn?
> > The question is whether .= stringifies its rhs in this case. If it > stringifies, passing an actual string to the lhs overload, then it is > correct to gripe about undef. If it does not stringify, and so passes > the rhs value to the lhs overload as-is, then it is up to the overload > method to determine what operand values are valid, and .= has no business > opining on the matter. > > In fact both 5.26.0 and 5.27.8 are agreed: .= does not stringify the rhs > in this case. They do not call stringification overloads, and do not > flatten references. They pass the rhs value to the lhs overload as-is.
Now fixed in blead with: commit af39014264c90cfc5a35e4f6e39ba038a8fb0c29 Author: David Mitchell <davem@iabyn.com> AuthorDate: Sat Feb 10 15:27:59 2018 +0000 Commit: David Mitchell <davem@iabyn.com> CommitDate: Mon Feb 19 22:06:49 2018 +0000 redo magic/overload handing in pp_multiconcat The way pp_multiconcat handles things like tieing and overloading doesn't work very well at the moment. There's a lot of code to handle edge cases, and there are still open bugs. The basic algorithm in pp_multiconcat is to first stringify (i.e. call SvPV() on) *all* args, then use the obtained values to calculate the total length and utf8ness required, then do a single SvGROW and copy all the bytes from all the args. This ordering is wrong when variables with visible side effects, such as tie/overload, are encountered. The current approach is to stringify args up until such an arg is encountered, concat all args up until that one together via the normal fast route, then jump to a special block of code which concats any remaining args one by one the "hard" way, handling overload etc. This is problematic because we sometimes need to go back in time. For example in ($undef . $overloaded), we're supposed to call $overloaded->concat($undef, reverse=1) so to speak, but by the time of the method call, we've already tried to stringify $undef and emitted a spurious 'uninit var' warning. The new approach taken in this commit is to: 1) Bail out of the stringify loop under a greater range of problematical variable classes - namely we stop when encountering *anything* which might cause external effects, so in addition to tied and overloaded vars, we now stop for any sort of get magic, or any undefined value where warnings are in scope. 2) If we bail out, we throw away any stringification results so far, and concatenate *all* args the slow way, even ones we're already stringified. This solves the "going back in time" problem mentioned above. It's safe because the only vars that get processed twice are ones for which the first stringification could have no side effects. The slow concat loop now uses S_do_concat(), which is a new static inline function which implements the main body of pp_concat() - so they share identical code. An intentional side-effect of this commit is to fix three tickets: RT #132783 RT #132827 RT #132595 so tests for them are included in this commit. One effect of this commit is that string concatenation of magic or undefined vars will now be slower than before, e.g. "pid=$$" "value=$undef" but they will probably still be faster than before pp_multiconcat was introduced. -- print+qq&$}$"$/$s$,$a$d$g$s$@$.$q$,$:$.$q$^$,$@$a$~$;$.$q$m&if+map{m,^\d{0\,},,${$::{$'}}=chr($"+=$&||1)}q&10m22,42}6:17a2~2.3@3;^2dg3q/s"&=~m*\d\*.*g


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org