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

Owner: Nobody
Requestors: rvtol [at] xs4all.nl
Cc:
AdminCc:

Operating System: freebsd
PatchStatus: (no value)
Severity: low
Type: core
Perl Version: 5.8.6
Fixed In: (no value)



CC: rj [...] xs4all.nl
Subject: return 0 or die;
Date: Sat, 11 Oct 2008 14:55:51 +0200 (CEST)
To: perlbug [...] perl.org
From: "Ruud H.G. van Tol" <rvtol [...] xs4all.nl>
Download (untitled) / with headers
text/plain 3.6k
This is a bug report for perl from rvtol@xs4all.nl, generated with the help of perlbug 1.35 running under perl v5.8.6. ----------------------------------------------------------------- [Please enter your report here] C<return doit() or die "did it wrong";> Would be nice if that warns "unreachable statement". [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl v5.8.6: Configured by rj at Thu Feb 3 20:42:58 CET 2005. Summary of my perl5 (revision 5 version 8 subversion 6) configuration: Platform: osname=freebsd, osvers=4.10-release-p2, archname=i386-freebsd-64int uname='freebsd xs0.xs4all.nl 4.10-release-p2 freebsd 4.10-release-p2 #1: thu aug 12 12:43:13 cest 2004 kai@xs0.xs4all.nl:usrobjusrsrcsysgeneric i386 ' config_args='-sde -Dprefix=/usr/local -Darchlib=/usr/local/lib/perl5/5.8.6/mach -Dprivlib=/usr/local/lib/perl5/5.8.6 -Dman3dir=/usr/local/lib/perl5/5.8.6/perl/man/man3 -Dman1dir=/usr/local/man/man1 -Dsitearch=/usr/local/lib/perl5/site_perl/5.8.6/mach -Dsitelib=/usr/local/lib/perl5/site_perl/5.8.6 -Dscriptdir=/usr/local/bin -Dsiteman3dir=/usr/local/lib/perl5/5.8.6/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Ui_malloc -Ui_iconv -Uinstallusrbinperl -Dcc=cc -Doptimize=-O -pipe -Duseshrplib -Dccflags=-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.6/BSDPAN" -Ud_dosuid -Ui_gdbm -Dusethreads=n -Dusemymalloc=y -Duse64bitint' 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=undef usemymalloc=y, bincompat5005=undef Compiler: cc='cc', ccflags ='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.6/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include', optimize='-O -pipe ', cppflags='-DAPPLLIB_EXP="/usr/local/lib/perl5/5.8.6/BSDPAN" -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='2.95.4 20020320 [FreeBSD]', 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='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -Wl,-E -L/usr/local/lib' libpth=/usr/lib /usr/local/lib libs=-lgdbm -lm -lcrypt -lutil perllibs=-lm -lcrypt -lutil libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' -Wl,-R/usr/local/lib/perl5/5.8.6/mach/CORE' cccdlflags='-DPIC -fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: SUIDPERLIO0 - fix PERLIO_DEBUG local root exploit (CAN-2005-0155) SUIDPERLIO1 - fix PERLIO_DEBUG buffer overflow (CAN-2005-0156) --- @INC for perl v5.8.6: /home/r/rvtol/.cpan/lib/i386-freebsd-64int /home/r/rvtol/.cpan/lib /usr/local/lib/perl5/site_perl/5.8.6/mach /usr/local/lib/perl5/site_perl/5.8.6 /usr/local/lib/perl5/site_perl /usr/local/lib/perl5/5.8.6/BSDPAN /usr/local/lib/perl5/5.8.6/mach /usr/local/lib/perl5/5.8.6 . --- Environment for perl v5.8.6: HOME=/home/r/rvtol LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH=/home/r/rvtol/bin/rrd/lib LOGDIR (unset) PATH=/home/r/rvtol/bin:/usr/local/bin:/bin:/usr/bin:/usr/games:. PERL5LIB=:/home/r/rvtol/.cpan/lib PERL_BADLANG (unset) SHELL=/usr/local/bin/bash
Subject: Re: [perl #59802] return 0 or die;
Date: Sun, 12 Oct 2008 22:48:09 +0200
To: perl5-porters [...] perl.org
From: "Dr.Ruud" <rvtol+news [...] isolution.nl>
Download (untitled) / with headers
text/plain 355b
"rvtol@xs4all.nl (via RT)" schreef: Show quoted text
> return doit() or die "did it wrong"; > > Would be nice if that warns "unreachable statement".
Another variant of "unreachable": perl -wle 'my $x = <>; $x++ or die; print $x' perl -wle 'my $x = <>; $x-- or die; print $x' (am not sure about the relation yet) -- Affijn, Ruud "Gewoon is een tijger."
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Sun, 12 Oct 2008 23:42:58 +0200
To: "Dr.Ruud" <rvtol+news [...] isolution.nl>
From: Eirik Berg Hanssen <Eirik-Berg.Hanssen [...] allverden.no>
Download (untitled) / with headers
text/plain 1018b
"Dr.Ruud" <rvtol+news@isolution.nl> writes: Show quoted text
> "rvtol@xs4all.nl (via RT)" schreef: > >
>> return doit() or die "did it wrong"; >> >> Would be nice if that warns "unreachable statement".
> > Another variant of "unreachable": > > perl -wle 'my $x = <>; $x++ or die; print $x' > perl -wle 'my $x = <>; $x-- or die; print $x'
How so unreachable? echo -n | perl -wle 'my $x = <>; $x++ or die; print $x' echo -n 0 | perl -wle 'my $x = <>; $x++ or die; print $x' echo 0 | perl -wle 'my $x = <>; $x++ or die; print $x' echo -n | perl -wle 'my $x = <>; $x-- or die; print $x' echo -n 0 | perl -wle 'my $x = <>; $x-- or die; print $x' echo 0 | perl -wle 'my $x = <>; $x-- or die; print $x' Died at -e line 1. Died at -e line 1, <> line 1. 1 Died at -e line 1. Died at -e line 1, <> line 1. -1 Show quoted text
> (am not sure about the relation yet)
(?) Eirik -- You just paddle around there awhile, and I'll explain about these poles ... -- Sally Brown Is that in Europe? -- Joyce Brown
Subject: Re: [perl #59802] return 0 or die;
Date: Sun, 12 Oct 2008 23:51:58 +0200
To: perl5-porters [...] perl.org
From: "Dr.Ruud" <rvtol+news [...] isolution.nl>
Download (untitled) / with headers
text/plain 310b
Eirik Berg Hanssen schreef: Show quoted text
> Dr.Ruud:
Show quoted text
>> Another variant of "unreachable": >> >> perl -wle 'my $x = <>; $x++ or die; print $x' >> perl -wle 'my $x = <>; $x-- or die; print $x'
> > How so unreachable?
Oops, please ignore. (forgot to chomp my test values) -- Affijn, Ruud "Gewoon is een tijger."
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 22 Oct 2008 01:19:52 +0200
To: perl5-porters [...] perl.org
From: "Dr.Ruud" <rvtol+news [...] isolution.nl>
Download (untitled) / with headers
text/plain 550b
"rvtol(AT)xs4all.nl (via RT)" schreef: Show quoted text
> C<return doit() or die "did it wrong";> > > Would be nice if that warns "unreachable statement".
Some examples can be seen here: http://www.google.com/codesearch?q=+lang:perl+\+return\+.*\+or\+die http://www.google.com/codesearch?q=lang:perl+return\+\S%2B\s%2Bor There is one in: http://search.cpan.org/src/GRICHTER/ExtUtils-XSBuilder-0.28/XSBuilder/ParseSource.pm A multi-line example: http://search.cpan.org/~markf/Attempt-1.01/lib/Sub/Attempts.pm -- Affijn, Ruud "Gewoon is een tijger."
CC: "Perl 5 Porters" <perl5-porters [...] perl.org>, ALEXMASS [...] cpan.org
Subject: Re: [perl #59802] return 0 or die;
Date: Thu, 23 Oct 2008 12:43:37 -0500
To: Dr.Ruud <rvtol+news [...] isolution.nl>
From: "David Nicol" <davidnicol [...] gmail.com>
Download (untitled) / with headers
text/plain 1.1k
this one looks like a typical offender: <<COPIED BatchSystem-SBS-0.32/lib/BatchSystem/SBS/Common.pm - 3 identical 88: if($simpleLocker){ return $simpleLocker->trylock($f) or CORE::die "cannot lock [$f]: $!"; }else{ 97: if($simpleLocker){ return $simpleLocker->unlock($f) or CORE::die "cannot lock [$f]: $!"; }else{ www.cpan.org/authors/id/A/AL/ALEXMASS/BatchSystem-SBS-0.32.tar.gz COPIED Refusing to allow return in non-void contexts would also disallow legitmate uses such as (wearedone and return) or domore; but how legit is that? wearedone and return; domore; would be a non-violating rewrite. Alternately, dropping the precedence of return would make C<return something or die> do what the coder wants. a low-precedence return would surely expose some bugs in currently unreachable code, would it break anything that is written well? On Tue, Oct 21, 2008 at 6:19 PM, Dr.Ruud <rvtol+news@isolution.nl> wrote: Show quoted text
>> Would be nice if that warns "unreachable statement".
> > Some examples can be seen here: > http://www.google.com/codesearch?q=+lang:perl+\+return\+.*\+or\+die
CC: "Dr.Ruud" <rvtol+news [...] isolution.nl>, Perl 5 Porters <perl5-porters [...] perl.org>, ALEXMASS [...] cpan.org
Subject: Re: [perl #59802] return 0 or die;
Date: Thu, 23 Oct 2008 23:44:59 +0200
To: David Nicol <davidnicol [...] gmail.com>
From: Alexandre Masselot <olav [...] genebio.com>
Download (untitled) / with headers
text/plain 1.2k
Hello David, I do not understand the problem.,, That's true that I was supposing the precedence of the return. I slightly modified the code as $simpleLocker->trylock($f) and return 1; die "cannot lock [$f]: $!"; would that make this piece of code more secure? to what extent? cheers Alex David Nicol wrote: Show quoted text
> this one looks like a typical offender: > > <<COPIED > BatchSystem-SBS-0.32/lib/BatchSystem/SBS/Common.pm - 3 identical > > 88: if($simpleLocker){ > return $simpleLocker->trylock($f) or CORE::die "cannot > lock [$f]: $!"; > }else{ > > > wearedone and return; > domore; > > would be a non-violating rewrite. > > Alternately, dropping the precedence of return would make C<return > something or die> do what the coder wants. > > a low-precedence return would surely expose some bugs in currently > unreachable code, would it break anything that is written well? > > > > On Tue, Oct 21, 2008 at 6:19 PM, Dr.Ruud <rvtol+news@isolution.nl> wrote: >
>>> Would be nice if that warns "unreachable statement". >>>
>> Some examples can be seen here: >> http://www.google.com/codesearch?q=+lang:perl+\+return\+.*\+or\+die >>
-- Alexandre Masselot, phD Senior bioinformatician www.genebio.com voice: +41 22 702 99 00
CC: David Nicol <davidnicol [...] gmail.com>, "Dr.Ruud" <rvtol+news [...] isolution.nl>, Perl 5 Porters <perl5-porters [...] perl.org>, ALEXMASS [...] cpan.org
Subject: Re: [perl #59802] return 0 or die;
Date: Fri, 24 Oct 2008 15:42:43 -0600
To: Alexandre Masselot <olav [...] genebio.com>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 117b
Return *is* low precedence already. Compare: return ($a + $b) * $c; with print ($a + $b) * $c; --tom
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 425b
I reviewed this 5-year-old ticket tonight. I don't see any claim that there is a bug in perl, nor do I see a request for a change in documentation or source code. All I see is some back and forth that would better be handled on IRC or on perlmonks. I am taking this ticket for the purpose of closing it in seven days -- unless someone can see something I haven't and takes the ticket over. Thank you very much. Jim Keenan
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 03:03:43 +0100
To: perlbug-followup [...] perl.org
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 712b
On 19 February 2013 02:51, James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text
> I reviewed this 5-year-old ticket tonight. I don't see any claim that > there is a bug in perl, nor do I see a request for a change in > documentation or source code. All I see is some back and forth that > would better be handled on IRC or on perlmonks. > > I am taking this ticket for the purpose of closing it in seven days -- > unless someone can see something I haven't and takes the ticket over.
Well, Im not going to take it over, but IMO: return $x or die "..."; should warn or something. So maybe the ticket should be marked stalled and not closed? Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 11:44:21 +0100
To: perl5-porters [...] perl.org, demerphq <demerphq [...] gmail.com>
From: "Dr.Ruud" <rvtol+usenet [...] isolution.nl>
Download (untitled) / with headers
text/plain 881b
On 2013-02-19 03:03, demerphq wrote: Show quoted text
> On 19 February 2013 02:51, James E Keenan via RT > <perlbug-followup@perl.org> wrote:
Show quoted text
>> I reviewed this 5-year-old ticket tonight. I don't see any claim that >> there is a bug in perl, nor do I see a request for a change in >> documentation or source code. All I see is some back and forth that >> would better be handled on IRC or on perlmonks. >> >> I am taking this ticket for the purpose of closing it in seven days -- >> unless someone can see something I haven't and takes the ticket over.
> > Well, Im not going to take it over, but IMO: > > return $x > or die "..."; > > should warn or something. > > So maybe the ticket should be marked stalled and not closed?
Indeed, the requested change is to warn on popular kinds of unreachable code, for example: return ... or ...; Or leave it to perlcritic/perltidy? -- Ruud
CC: perl5-porters [...] perl.org, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 10:53:47 +0000
To: "Dr.Ruud" <rvtol+usenet [...] isolution.nl>
From: Nicholas Clark <nick [...] ccl4.org>
Download (untitled) / with headers
text/plain 736b
On Tue, Feb 19, 2013 at 11:44:21AM +0100, Dr.Ruud wrote: Show quoted text
> On 2013-02-19 03:03, demerphq wrote:
Show quoted text
> > Well, Im not going to take it over, but IMO: > > > > return $x > > or die "..."; > > > > should warn or something. > > > > So maybe the ticket should be marked stalled and not closed?
> > Indeed, the requested change is to warn on popular kinds of unreachable > code, for example: > > return ... or ...;
(subexpression that always causes control flow) [operator] ... where ... is unreachable. Show quoted text
> Or leave it to perlcritic/perltidy?
The code that we're suggesting should warn, is buggy as written. I don't think that that's really a "tidy" issue, or even a style issue. To my mind, it's a legitimate todo. Nicholas Clark
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 13:12:42 +0100
To: demerphq <demerphq [...] gmail.com>
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 306b
On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote: Show quoted text
> Well, Im not going to take it over, but IMO: > > return $x > or die "..."; > > should warn or something. > > So maybe the ticket should be marked stalled and not closed?
Yeah, this is always buggy, so we might as well warn. Leon
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 15:08:34 +0100
To: perl5-porters [...] perl.org
From: Aristoteles Pagaltzis <pagaltzis [...] gmx.de>
Download (untitled) / with headers
text/plain 397b
* Leon Timmermans <fawaka@gmail.com> [2013-02-19 13:15]: Show quoted text
> Yeah, this is always buggy, so we might as well warn.
Except in Perl poetry. I am sure that Paul Fenwick, for one, would appreciate `return 1 or die`. :-) </digression> -- *AUTOLOAD=*_;sub _{s/::([^:]*)$/print$1,(",$\/"," ")[defined wantarray]/e;chop;$_} &Just->another->Perl->hack; #Aristotle Pagaltzis // <http://plasmasturm.org/>
CC: demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 09:59:58 -0500
To: Leon Timmermans <fawaka [...] gmail.com>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 375b
* Leon Timmermans <fawaka@gmail.com> [2013-02-19T07:12:42] Show quoted text
> On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
> > return $x > > or die "..."; > > > > should warn or something.
> Yeah, this is always buggy, so we might as well warn.
Agreed. I've made this mistake and then wondered why I didn't get an unreachable code warning, in the past. -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

CC: perl5-porters [...] perl.org, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 08:50:42 -0700
To: "Dr.Ruud" <rvtol+usenet [...] isolution.nl>
From: Tom Christiansen <tchrist [...] perl.com>
"Dr.Ruud" <rvtol+usenet@isolution.nl> wrote on Tue, 19 Feb 2013 11:44:21 +0100: Show quoted text
>On 2013-02-19 03:03, demerphq wrote:
>> On 19 February 2013 02:51, James E Keenan via RT >> <perlbug-followup@perl.org> wrote:
Show quoted text
>>> I reviewed this 5-year-old ticket tonight. I don't see any claim that >>> there is a bug in perl, nor do I see a request for a change in >>> documentation or source code. All I see is some back and forth that >>> would better be handled on IRC or on perlmonks. >>> >>> I am taking this ticket for the purpose of closing it in seven days -- >>> unless someone can see something I haven't and takes the ticket over.
>> >> Well, Im not going to take it over, but IMO: >> >> return $x >> or die "..."; >> >> should warn or something. >> >> So maybe the ticket should be marked stalled and not closed?
Show quoted text
>Indeed, the requested change is to warn on popular kinds of unreachable >code, for example:
Show quoted text
> return ... or ...;
Show quoted text
>Or leave it to perlcritic/perltidy?
What is it that gardenpaths people into making this sort of error, anyway? I'm serious. --tom
CC: demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 08:51:42 -0700
To: Leon Timmermans <fawaka [...] gmail.com>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 629b
Leon Timmermans <fawaka@gmail.com> wrote on Tue, 19 Feb 2013 13:12:42 +0100: Show quoted text
>On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
>> Well, Im not going to take it over, but IMO: >> >> return $x >> or die "..."; >> >> should warn or something. >> >> So maybe the ticket should be marked stalled and not closed?
Show quoted text
>Yeah, this is always buggy, so we might as well warn.
Precedence is different on return than anywhere else in Perl. Try explaining why this elicits warning 1: sub t1 { return(0) || warn "warning 1" } sub t2 { return(0) or warn "warning 2" } $x = t1(); $y = t2(); --tom
CC: Leon Timmermans <fawaka [...] gmail.com>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 09:02:02 -0700
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 571b
Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote on Tue, 19 Feb 2013 09:59:58 EST: Show quoted text
>* Leon Timmermans <fawaka@gmail.com> [2013-02-19T07:12:42]
>> On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
Show quoted text
>>> return $x >>> or die "..."; >>> >>> should warn or something.
Show quoted text
>> Yeah, this is always buggy, so we might as well warn.
Show quoted text
> Agreed. I've made this mistake and then wondered why I > didn't get an unreachable code warning, in the past.
Again I ask: what is it that leads anyone ever to make that particular error? I do not understand. --tom
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, Leon Timmermans <fawaka [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 17:10:14 +0100
To: Tom Christiansen <tchrist [...] perl.com>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 839b
On 19 February 2013 17:02, Tom Christiansen <tchrist@perl.com> wrote: Show quoted text
> Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote > on Tue, 19 Feb 2013 09:59:58 EST: >
>>* Leon Timmermans <fawaka@gmail.com> [2013-02-19T07:12:42]
>>> On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
>
>>>> return $x >>>> or die "..."; >>>> >>>> should warn or something.
>
>>> Yeah, this is always buggy, so we might as well warn.
>
>> Agreed. I've made this mistake and then wondered why I >> didn't get an unreachable code warning, in the past.
> > Again I ask: what is it that leads anyone ever to make that > particular error? I do not understand.
Who cares? People do it and it causes trouble and Perl should probably be able to figure out how to warn about it. cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, Leon Timmermans <fawaka [...] gmail.com>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 13:55:45 -0300
To: Tom Christiansen <tchrist [...] perl.com>
From: Brian Fraser <fraserbn [...] gmail.com>
Download (untitled) / with headers
text/plain 801b
On Tue, Feb 19, 2013 at 1:02 PM, Tom Christiansen <tchrist@perl.com> wrote: Show quoted text
> Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote > on Tue, 19 Feb 2013 09:59:58 EST: >
>>* Leon Timmermans <fawaka@gmail.com> [2013-02-19T07:12:42]
>>> On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
>
>>>> return $x >>>> or die "..."; >>>> >>>> should warn or something.
>
>>> Yeah, this is always buggy, so we might as well warn.
>
>> Agreed. I've made this mistake and then wondered why I >> didn't get an unreachable code warning, in the past.
> > Again I ask: what is it that leads anyone ever to make that > particular error? I do not understand. >
Playing the devil's advocate, how about: Blind code standardization, the sort that goes "every function should have an explicit return!"
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, Leon Timmermans <fawaka [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 11:25:29 -0700
To: demerphq <demerphq [...] gmail.com>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 1.3k
demerphq <demerphq@gmail.com> wrote on Tue, 19 Feb 2013 17:10:14 +0100: Show quoted text
>On 19 February 2013 17:02, Tom Christiansen <tchrist@perl.com> wrote:
>> Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote >> on Tue, 19 Feb 2013 09:59:58 EST: >>
>>>* Leon Timmermans <fawaka@gmail.com> [2013-02-19T07:12:42]
>>>> On Tue, Feb 19, 2013 at 3:03 AM, demerphq <demerphq@gmail.com> wrote:
>>
>>>>> return $x >>>>> or die "..."; >>>>> >>>>> should warn or something.
>>
>>>> Yeah, this is always buggy, so we might as well warn.
>>
>>> Agreed. I've made this mistake and then wondered why I >>> didn't get an unreachable code warning, in the past.
>> >> Again I ask: what is it that leads anyone ever to make that >> particular error? I do not understand.
Show quoted text
> Who cares? People do it and it causes trouble and Perl should probably > be able to figure out how to warn about it.
The reason it matters is because I have no idea what that is supposed to mean. There are uncountably many similar constructs that Perl does not warn on either, like $ perl -cwe 'die "ABC"; die "XYZ"' -e syntax OK $ perl -cwe 'die("ABC") || die("XYZ")' -e syntax OK $ perl -cwe 'die("ABC") && die("XYZ")' -e syntax OK None of which are caught and all of which should be. Remember /* NOTREACHED */ I want to understand what it is that the people who make this error were thinking, and how they came to think that. --tom
CC: demerphq <demerphq [...] gmail.com>, Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 19:28:44 +0100
To: Tom Christiansen <tchrist [...] perl.com>
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 701b
On Tue, Feb 19, 2013 at 7:25 PM, Tom Christiansen <tchrist@perl.com> wrote: Show quoted text
> The reason it matters is because I have no idea what that is supposed > to mean. > > There are uncountably many similar constructs that Perl does not warn > on either, like > > $ perl -cwe 'die "ABC"; die "XYZ"' > -e syntax OK > > $ perl -cwe 'die("ABC") || die("XYZ")' > -e syntax OK > > $ perl -cwe 'die("ABC") && die("XYZ")' > -e syntax OK > > None of which are caught and all of which should be. Remember > > /* NOTREACHED */ > > I want to understand what it is that the people who make this > error were thinking, and how they came to think that.
It's a simple precedence mistake. Leon
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 21:48:26 +0100
To: perl5-porters [...] perl.org
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 279b
On Tue, Feb 19, 2013 at 3:08 PM, Aristoteles Pagaltzis <pagaltzis@gmx.de> wrote: Show quoted text
> Except in Perl poetry. I am sure that Paul Fenwick, for one, > would appreciate `return 1 or die`. :-) </digression>
Fortunately, one usually doesn't run poetry with warnings enabled ;-) Leon
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 20 Feb 2013 00:06:02 +0100
To: perl5-porters [...] perl.org
From: Aristotle Pagaltzis <pagaltzis [...] gmx.de>
Download (untitled) / with headers
text/plain 1.3k
* Tom Christiansen <tchrist@perl.com> [2013-02-19 17:00]: Show quoted text
> Precedence is different on return than anywhere else in Perl. > Try explaining why this elicits warning 1: > > sub t1 { return(0) || warn "warning 1" } > sub t2 { return(0) or warn "warning 2" } > $x = t1(); > $y = t2();
This smells to me like a super-special parse reminiscent of how Perl parses sort foo($x) which has cost me way too many hairs over the years. I run into it just rarely enough to never suspect the real problem, not only because the rarity means I forget, but also because it almost invariably happens in a chain of data structure-munging list ops, so the symptom is a weird number of elements returned at the end, and at some very surprising deref level. So I end up on a long goose chase of conjectures every time before I finally start converging on the real problem and eventually remember with a start that `sort` is hatefully super-special. Ugh. Err. I’m afraid I’m not sure any of that is even relevant to your point… all I have is the suspicion that this is a similar case. But I feel a need to rant about `sort` whenever I am reminded of this aspect of it; see above for why. -- *AUTOLOAD=*_;sub _{s/::([^:]*)$/print$1,(",$\/"," ")[defined wantarray]/e;chop;$_} &Just->another->Perl->hack; #Aristotle Pagaltzis // <http://plasmasturm.org/>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 17:41:41 -0700
To: Aristotle Pagaltzis <pagaltzis [...] gmx.de>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 1.7k
Aristotle Pagaltzis <pagaltzis@gmx.de> wrote on Wed, 20 Feb 2013 00:06:02 +0100: Show quoted text
> * Tom Christiansen <tchrist@perl.com> [2013-02-19 17:00]:
> > Precedence is different on return than anywhere else in Perl. > > Try explaining why this elicits warning 1: > > > > sub t1 { return(0) || warn "warning 1" } > > sub t2 { return(0) or warn "warning 2" } > > $x = t1(); > > $y = t2();
Show quoted text
> This smells to me like a super-special parse reminiscent of how Perl > parses
Show quoted text
> sort foo($x)
Show quoted text
> which has cost me way too many hairs over the years. I run into it just > rarely enough to never suspect the real problem, not only because the > rarity means I forget, but also because it almost invariably happens in > a chain of data structure-munging list ops, so the symptom is a weird > number of elements returned at the end, and at some very surprising > deref level. So I end up on a long goose chase of conjectures every time > before I finally start converging on the real problem and eventually > remember with a start that `sort` is hatefully super-special. Ugh.
Show quoted text
> Err.
Show quoted text
> I’m afraid I’m not sure any of that is even relevant to your point… all > I have is the suspicion that this is a similar case. But I feel a need > to rant about `sort` whenever I am reminded of this aspect of it; see > above for why.
The thing about return is that it is design to make it hard to make precedence errors with. You can even return ($a + 5) * 9; and have it do the right thing. I just had never heard of anybody making a precedence error with it before, because it tries so hard to make that impossible. Leave it that skunky "or" thing to screw that up. Thank goodness I never used it, and so never have precedence problems. :) --tom
CC: Leon Timmermans <fawaka [...] gmail.com>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 19:54:01 -0500
To: Tom Christiansen <tchrist [...] perl.com>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 671b
* Tom Christiansen <tchrist@perl.com> [2013-02-19T11:02:02] Show quoted text
> Again I ask: what is it that leads anyone ever to make that > particular error? I do not understand.
I don't remember how it came about. I assume it was the result of absent-minded refactoring, where: my $x = foo or die; return $x; turned into return foo or die; ...but I can't remember. I only remember that I've gotten that sort of nonsense in my code. If I find evidence of commits fixing it or, Heaven forbid, I make that mistake again, I'll try to let you know how it came about. I do think it's useful to know what kind of confusion of ideas that could provoke such a mistake. -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

CC: Tom Christiansen <tchrist [...] perl.com>, Leon Timmermans <fawaka [...] gmail.com>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 17:07:05 -0800
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
From: Jan Dubois <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 552b
On Tue, Feb 19, 2013 at 4:54 PM, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote: Show quoted text
> > I don't remember how it came about. I assume it was the result of > absent-minded refactoring, where: > > my $x = foo or die; > return $x; > > turned into > > return foo or die;
Just for the record, I've also seen return foo() and bar(); # supposed to return TRUE only when both foo() and bar() are true in the wild (can't remember where). Which I guess proves Tom's point that adding and/or didn't make things safer for beginners. Cheers, -Jan
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, Tom Christiansen <tchrist [...] perl.com>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 20 Feb 2013 02:51:49 +0100
To: Jan Dubois <jand [...] activestate.com>
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 306b
On Wed, Feb 20, 2013 at 2:07 AM, Jan Dubois <jand@activestate.com> wrote: Show quoted text
> in the wild (can't remember where). Which I guess proves Tom's point > that adding and/or didn't make things safer for beginners.
People write bugs on any precedence level, I don't think that that argument is convincing. Leon
CC: Jan Dubois <jand [...] activestate.com>, Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 19 Feb 2013 20:11:15 -0700
To: Leon Timmermans <fawaka [...] gmail.com>
From: Tom Christiansen <tchrist [...] perl.com>
Download (untitled) / with headers
text/plain 1.6k
Leon Timmermans <fawaka@gmail.com> wrote on Wed, 20 Feb 2013 02:51:49 +0100: Show quoted text
>On Wed, Feb 20, 2013 at 2:07 AM, Jan Dubois <jand@activestate.com> wrote:
>> in the wild (can't remember where). Which I guess proves Tom's point >> that adding and/or didn't make things safer for beginners.
Show quoted text
> People write bugs on any precedence level, I don't think that that > argument is convincing.
The argument is that the only people who do not make precedence mistakes are the people who use parentheses. You are not convinced? Thinking that you -- and everybody else! -- always know, remember, and correctly apply all 24 (twenty-four) different precedence levels in all situations without using parens is not realistic, and it leads to error. 1: left terms and list operators (leftward) 2: left -> 3: nonassoc ++ -- 4: right ** 5: right ! ~ \ and unary + and - 6: left =~ !~ 7: left * / % x 8: left + - . 9: left << >> 10: nonassoc named unary operators 11: nonassoc < > <= >= lt gt le ge 12: nonassoc == != <=> eq ne cmp ~~ 13: left & 14: left | ^ 15: left && 16: left || // 17: nonassoc .. ... 18: right ?: 19: right = += -= *= etc. 20: left , => 21: nonassoc list operators (rightward) 22: right not 23: left and 24: left or xor That is the reasoning behind "just use parens". Once you start doing that, all of these precedence "problems" disappear. And so do the funny bits at the bottom of the table. :) --tom
CC: Jan Dubois <jand [...] activestate.com>, Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, demerphq <demerphq [...] gmail.com>, perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 26 Feb 2013 20:40:40 +0100
To: Tom Christiansen <tchrist [...] perl.com>
From: Leon Timmermans <fawaka [...] gmail.com>
Download (untitled) / with headers
text/plain 1.1k
On Wed, Feb 20, 2013 at 4:11 AM, Tom Christiansen <tchrist@perl.com> wrote: Show quoted text
>> People write bugs on any precedence level, I don't think that that >> argument is convincing.
> > The argument is that the only people who do not make precedence mistakes > are the people who use parentheses. You are not convinced?
Indeed I am (not convinced) that ((parentheses) always (make code (more readable))). Specially not when you're using multiple sets of parentheses. The precedence levels are such that most of the time you don't need any parentheses. IMHO they're extra noise in your code. They add to the mental effort required for parsing. In case of C<or> and C<and>, understanding In case of C<or> and C<and>, understanding they're the absolute lowest levels of precedence will make things much easier to read. Show quoted text
> Thinking that you -- and everybody else! -- always know, remember, and > correctly apply all 24 (twenty-four) different precedence levels in > all situations without using parens is not realistic, and it leads > to error.
To me that sounds like optimizing writability over readability. Then again, style discussions are worst then bike-shedding, there is no ultimate truth, etcetera… Leon
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 27 Feb 2013 03:27:38 -0600
To: Tom Christiansen <tchrist [...] perl.com>
From: David Nicol <davidnicol [...] gmail.com>
Download (untitled) / with headers
text/plain 1.4k


On Tue, Feb 19, 2013 at 9:11 PM, Tom Christiansen <tchrist@perl.com> wrote: Show quoted text
    21: nonassoc    list operators (rightward)
    22: right       not
    23: left        and
    24: left        or xor

That is the reasoning behind "just use parens".  Once you start doing that,
all of these precedence "problems" disappear.  And so do the funny bits
at the bottom of the table. :)

--tom

question:
would adding a 25th level for C<return> break anything, aside from these C<return something verbose-short-circuiter somethingelse> instances that currently happen to be in production and working anyway? Maybe C<die> could go there too?

answer:
yes, that would break any code that was lazily written by inserting a return or a die within a chain of C<and> operators
where the coder didn't bother to comment out all the unreachable ones: the code would suddenly return (or throw) different results.

conclusion:  
it's a feature.

example:
 given a routine like so:

         sub check_for_various_conditions{
                critical_condition or
                severe_condition or
                medium_condition or
                mild_condition or
                0
         }

and the coder who wrote it doesn't want to care about mild conditions any more, at least during
the current phase of the development cycle.

Instead of commenting out the mild_condition line, someone who proudly understands Perl precedence
decides to show off and insert "return 0 or" above it.

Does p5p support that coder?

Subject: Re: [perl #59802] return 0 or die;
Date: Sun, 03 Mar 2013 04:01:56 +0100
To: perl5-porters [...] perl.org
From: Lukas Mai <l.mai [...] web.de>
Download (untitled) / with headers
text/plain 474b
On 19.02.2013 19:25, Tom Christiansen wrote: Show quoted text
> > There are uncountably many similar constructs that Perl does not warn > on either, like > > $ perl -cwe 'die "ABC"; die "XYZ"' > -e syntax OK > > $ perl -cwe 'die("ABC") || die("XYZ")' > -e syntax OK > > $ perl -cwe 'die("ABC") && die("XYZ")' > -e syntax OK
Add 'die die die "ABC"' and 'return return "XYZ"' to that list. Hey, die and return are idempotent. :-) -- Lukas Mai <l.mai@web.de>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Sun, 3 Mar 2013 00:37:03 -0300
To: Lukas Mai <l.mai [...] web.de>
From: Brian Fraser <fraserbn [...] gmail.com>
Download (untitled) / with headers
text/plain 642b
On Sun, Mar 3, 2013 at 12:01 AM, Lukas Mai <l.mai@web.de> wrote: Show quoted text
> On 19.02.2013 19:25, Tom Christiansen wrote:
>> >> >> There are uncountably many similar constructs that Perl does not warn >> on either, like >> >> $ perl -cwe 'die "ABC"; die "XYZ"' >> -e syntax OK >> >> $ perl -cwe 'die("ABC") || die("XYZ")' >> -e syntax OK >> >> $ perl -cwe 'die("ABC") && die("XYZ")' >> -e syntax OK
> > > Add 'die die die "ABC"' and 'return return "XYZ"' to that list. > > Hey, die and return are idempotent. :-) >
BEGIN { *CORE::GLOBAL::die = sub { return unless $really_meant_it++; goto &CORE::die } } die die !!!111;
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 29 Apr 2013 11:28:56 +0100
To: perlbug-followup [...] perl.org
From: Miles Gould <miles [...] assyrian.org.uk>
Download (untitled) / with headers
text/plain 585b
On 19 Feb 2013 at 17:07:05, Jan Dubois wrote: Show quoted text
> Just for the record, I've also seen > > return foo() and bar(); # supposed to return TRUE only when > both foo() and bar() are true
I ran into this problem last night. The offending line was return ($self->true_count($idx) > 0) xor ($self->false_count($idx) > 0); and in this case it's easy to explain what I had in mind: I wanted my function to return the logical xor of those two expressions! Once I'd tracked the problem down it was easy enough to fix by reparenthesising, but a warning would have been very useful. Miles
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 988b
On Tue Feb 19 02:54:23 2013, nicholas wrote: Show quoted text
> On Tue, Feb 19, 2013 at 11:44:21AM +0100, Dr.Ruud wrote:
> > On 2013-02-19 03:03, demerphq wrote:
>
> > > Well, Im not going to take it over, but IMO: > > > > > > return $x > > > or die "..."; > > > > > > should warn or something. > > > > > > So maybe the ticket should be marked stalled and not closed?
> > > > Indeed, the requested change is to warn on popular kinds of unreachable > > code, for example: > > > > return ... or ...;
> > (subexpression that always causes control flow) [operator] ... > > where ... is unreachable. >
> > Or leave it to perlcritic/perltidy?
> > The code that we're suggesting should warn, is buggy as written. I don't > think that that's really a "tidy" issue, or even a style issue. > > To my mind, it's a legitimate todo. > > Nicholas Clark >
Is there anyone actually willing to take on the job of creating a warning that will run when code is unreachable? Thank you very much. Jim Keenan
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 15 Jul 2013 23:00:00 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 958b
Show quoted text
> Is there anyone actually willing to take on the job of creating a > warning that will run when code is unreachable? > > Thank you very much. > Jim Keenan
Hi, Yes, I have a prototype patch (see attached). The warning is triggered in a couple of (cpan) modules included in perlcore. This list includes at least Test::Builder, Parse::CPAN::Meta and JSON::PP. Each of these three modules have received a bug report[1]. I do also see a couple of test failures from applying the patch. From what I can tell, they are all caused by the new warning showing up in the abovementioned modules. That said, the new warning does not cause all tests triggering it to fail, so I may I have missed a buggy module or two. Thanks for considering, ~Niels PS: Please CC [1] Test::Builder: https://github.com/schwern/test-more/pull/385 Parse::Cpan::Meta: https://rt.cpan.org/Ticket/Display.html?id=86947 JSON::PP: https://rt.cpan.org/Ticket/Display.html?id=86948

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
On Mon Jul 15 15:53:55 2013, niels@thykier.net wrote: Show quoted text
> > Is there anyone actually willing to take on the job of creating a > > warning that will run when code is unreachable? > > > > Thank you very much. > > Jim Keenan
> > Hi, > > Yes, I have a prototype patch (see attached). > > The warning is triggered in a couple of (cpan) modules included in > perlcore. This list includes at least Test::Builder, Parse::CPAN::Meta > and JSON::PP. Each of these three modules have received a bug report[1]. > > I do also see a couple of test failures from applying the patch. From > what I can tell, they are all caused by the new warning showing up in > the abovementioned modules. That said, the new warning does not cause > all tests triggering it to fail, so I may I have missed a buggy module > or two. > > Thanks for considering, > ~Niels
Thank you. The patch looks good (though I haven’t tested it yet). I have one quibble, though: perldiag’s messages are in alphabetical order, but the patch does not respect that. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 1.3k
On Mon Jul 15 18:20:35 2013, sprout wrote: Show quoted text
> On Mon Jul 15 15:53:55 2013, niels@thykier.net wrote:
> > > Is there anyone actually willing to take on the job of creating a > > > warning that will run when code is unreachable? > > > > > > Thank you very much. > > > Jim Keenan
> > > > Hi, > > > > Yes, I have a prototype patch (see attached). > > > > The warning is triggered in a couple of (cpan) modules included in > > perlcore. This list includes at least Test::Builder, Parse::CPAN::Meta > > and JSON::PP. Each of these three modules have received a bug
report[1]. Thank you. Show quoted text
> > > > I do also see a couple of test failures from applying the patch. From > > what I can tell, they are all caused by the new warning showing up in > > the abovementioned modules.
That is sufficient reason to wait until newer versions are released and then imported into perl.git before applying the patch. Show quoted text
> > That said, the new warning does not cause > > all tests triggering it to fail, so I may I have missed a buggy module > > or two. > > > > Thanks for considering, > > ~Niels
> > Thank you. The patch looks good (though I haven’t tested it yet). I > have one quibble, though: perldiag’s messages are in alphabetical > order, but the patch does not respect that.
One more thing: This could be extended to other ops that don’t return, such as last, goto, die, etc. -- Father Chrysostomos
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 16 Jul 2013 10:58:01 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 1.7k
On 2013-07-16 05:31, Father Chrysostomos via RT wrote: Show quoted text
> [...]
>> >> Thank you. The patch looks good (though I haven’t tested it yet). I >> have one quibble, though: perldiag’s messages are in alphabetical >> order, but the patch does not respect that.
>
Here is a revised patch. The warning has been inserted in its proper place in perldiag.pod. Show quoted text
> One more thing: This could be extended to other ops that don’t return, > such as last, goto, die, etc. >
The warning is now emitted for any of the following ops: OP_{NEXT,LAST,REDO,RETURN,DIE,EXIT,GOTO} I noticed and have documented that some possible intended uses of this precedence. An example being: use constant FEATURE => 0; sub { not FEATURE and return or do_feature(); } perl correctly parses this as: sub { (not FEATURE and return) or do_feature(); } but it still causes a warning. I am not sure how to detect this case, so I am not able to devise a fix for this false-positive (other than recommending that people add the parentheses themselves to express the intended grouping). The problem in detecting this is that by the time the warning is emitted, constant folding has eliminated the context. The steps are (something like): Start with "not FEATURE # and return or do_feature();". (The "#" denotes the part not parsed/examined yet). Fold "not FEATURE" to 1 leaving "1 # and return or do_feature();" Parse/Consume "and return" giving "1 and return # or do_feature();" Reduce "1 and return #or do_feature();" to "return # or do_feature();" Parse/Consume "or do_feature();" giving "return or do_feature();" Warn as "return" is on the left-hand side of a (logical) binary operator (and is not enclosed in parentheses). ~Niels

Message body is not shown because sender requested not to inline it.

CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Tue, 16 Jul 2013 09:01:09 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 1.1k
On 2013-07-16 05:31, Father Chrysostomos via RT wrote: Show quoted text
> [...]
>>> >>> I do also see a couple of test failures from applying the patch. From >>> what I can tell, they are all caused by the new warning showing up in >>> the abovementioned modules.
> > That is sufficient reason to wait until newer versions are released and > then imported into perl.git before applying the patch. >
Certainly. Show quoted text
>>> That said, the new warning does not cause >>> all tests triggering it to fail, so I may I have missed a buggy module >>> or two. >>> >>> Thanks for considering, >>> ~Niels
>> >> Thank you. The patch looks good (though I haven’t tested it yet). I >> have one quibble, though: perldiag’s messages are in alphabetical >> order, but the patch does not respect that.
> > One more thing: This could be extended to other ops that don’t return, > such as last, goto, die, etc. >
Seems reasonable, will look into it. [From previous mail] Show quoted text
> > Thank you. The patch looks good (though I haven’t tested it yet). I > have one quibble, though: perldiag’s messages are in alphabetical > order, but the patch does not respect that.
Ah, haven't noticed that. Will fix that in the next revision. ~Niels
RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 1.6k
On Tue Jul 16 13:35:28 2013, niels@thykier.net wrote: Show quoted text
> On 2013-07-16 05:31, Father Chrysostomos via RT wrote:
> > [...]
> >> > >> Thank you. The patch looks good (though I haven’t tested it yet). I > >> have one quibble, though: perldiag’s messages are in alphabetical > >> order, but the patch does not respect that.
> >
> > Here is a revised patch. > > The warning has been inserted in its proper place in perldiag.pod. >
> > One more thing: This could be extended to other ops that don’t return, > > such as last, goto, die, etc. > >
> > The warning is now emitted for any of the following ops: > OP_{NEXT,LAST,REDO,RETURN,DIE,EXIT,GOTO} > > I noticed and have documented that some possible intended uses of this > precedence. An example being: > > use constant FEATURE => 0; > sub { not FEATURE and return or do_feature(); } > > perl correctly parses this as: > > sub { (not FEATURE and return) or do_feature(); } > > but it still causes a warning. I am not sure how to detect this case, > so I am not able to devise a fix for this false-positive (other than > recommending that people add the parentheses themselves to express the > intended grouping).
I have an idea: We have three spare bits in op->op_spare. We could use one of them to indicate folding. We already set OPf_SPECIAL on regexp and tr operators in S_new_logop and newCONDOP to indicate this. We also set OPpCONST_FOLDED on constants in those two places. If we also set, say, op->op_folded to 1 in those same code paths, then we can detect the fold. (And we could simplify the code by having match-like ops and constants us the same bit.) -- Father Chrysostomos
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 17 Jul 2013 10:30:14 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 950b
On 2013-07-17 00:54, Father Chrysostomos via RT wrote: Show quoted text
> I have an idea: We have three spare bits in op->op_spare. We could use > one of them to indicate folding. >
That would be possible, indeed. Show quoted text
> We already set OPf_SPECIAL on regexp and tr operators in S_new_logop and > newCONDOP to indicate this. We also set OPpCONST_FOLDED on constants in > those two places. > > If we also set, say, op->op_folded to 1 in those same code paths, then > we can detect the fold. > > (And we could simplify the code by having match-like ops and constants > us the same bit.)
You wrote "also set op->op_folded" and later imply the change could simplify the code by using the same bit. Just to confirm here, did you want the "op_folded" member replace OPpCONST_FOLDED and (for regexp/tr operators) OPf_SPECIAL? And if so, should OPf_SPECIAL still be set in these cases since it is a public flag or should it be cleared so we can repurpose it? ~Niels
RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 1.3k
On Wed Jul 17 01:30:56 2013, niels@thykier.net wrote: Show quoted text
> On 2013-07-17 00:54, Father Chrysostomos via RT wrote:
> > I have an idea: We have three spare bits in op->op_spare. We could use > > one of them to indicate folding. > >
> > That would be possible, indeed. >
> > We already set OPf_SPECIAL on regexp and tr operators in S_new_logop and > > newCONDOP to indicate this. We also set OPpCONST_FOLDED on constants in > > those two places. > > > > If we also set, say, op->op_folded to 1 in those same code paths, then > > we can detect the fold. > > > > (And we could simplify the code by having match-like ops and constants > > us the same bit.)
> > You wrote "also set op->op_folded" and later imply the change could > simplify the code by using the same bit. > Just to confirm here, did you want the "op_folded" member replace > OPpCONST_FOLDED and (for regexp/tr operators) OPf_SPECIAL?
That would be nice. But it is not a prerequisite for getting your patch in. :-) Show quoted text
> And if so, > should OPf_SPECIAL still be set in these cases since it is a public flag > or should it be cleared so we can repurpose it?
I think (off the top of my head) that B::Deparse currently uses it. So B::Deparse may have to change. I don’t remember now. OPf_SPECIAL is a private flag, at least in terms of its use, but stored with the public flags. -- Father Chrysostomos
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 17 Jul 2013 21:36:15 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 1.3k
On 2013-07-17 18:56, Father Chrysostomos via RT wrote: Show quoted text
> On Wed Jul 17 01:30:56 2013, niels@thykier.net wrote:
>> [...] >> >> You wrote "also set op->op_folded" and later imply the change could >> simplify the code by using the same bit. >> Just to confirm here, did you want the "op_folded" member replace >> OPpCONST_FOLDED and (for regexp/tr operators) OPf_SPECIAL?
> > That would be nice. But it is not a prerequisite for getting your patch > in. :-) >
>> And if so, >> should OPf_SPECIAL still be set in these cases since it is a public flag >> or should it be cleared so we can repurpose it?
> > I think (off the top of my head) that B::Deparse currently uses it. So > B::Deparse may have to change. I don’t remember now. > > OPf_SPECIAL is a private flag, at least in terms of its use, but stored > with the public flags. >
Included is a patch to add the op_folded member and my previous patch has been rebased on top of it. RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally in the code (i.e. perl does the right thing even without it). However, I left it in (and kept it maintained) for now until B::Concise can be updated. I presume that will involve exposing op_folded via B.pm, which I have left as "future work" for now. I made no attempt to have it replace OPf_SPECIAL on the regexp/tr operators either. ~Niels

Message body is not shown because sender requested not to inline it.

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 1.5k
On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote: Show quoted text
> On 2013-07-17 18:56, Father Chrysostomos via RT wrote:
> > On Wed Jul 17 01:30:56 2013, niels@thykier.net wrote:
> >> [...] > >> > >> You wrote "also set op->op_folded" and later imply the change could > >> simplify the code by using the same bit. > >> Just to confirm here, did you want the "op_folded" member replace > >> OPpCONST_FOLDED and (for regexp/tr operators) OPf_SPECIAL?
> > > > That would be nice. But it is not a prerequisite for getting your patch > > in. :-) > >
> >> And if so, > >> should OPf_SPECIAL still be set in these cases since it is a public
flag Show quoted text
> >> or should it be cleared so we can repurpose it?
> > > > I think (off the top of my head) that B::Deparse currently uses it. So > > B::Deparse may have to change. I don’t remember now. > > > > OPf_SPECIAL is a private flag, at least in terms of its use, but stored > > with the public flags. > >
> > Included is a patch to add the op_folded member and my previous patch > has been rebased on top of it. > > RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally in the > code (i.e. perl does the right thing even without it). However, I left > it in (and kept it maintained) for now until B::Concise can be updated. > I presume that will involve exposing op_folded via B.pm, which I have > left as "future work" for now. > I made no attempt to have it replace OPf_SPECIAL on the regexp/tr > operators either.
Those patches look good. Thank you. Now we just have to wait for Parse::CPAN::Meta. -- Father Chrysostomos
CC: p5p <perl5-porters [...] perl.org>, niels [...] thykier.net
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 17 Jul 2013 16:59:41 -0400
To: James E Keenan via RT <perlbug-followup [...] perl.org>
From: David Golden <xdg [...] xdg.me>
Download (untitled) / with headers
text/plain 375b
On Wed, Jul 17, 2013 at 4:12 PM, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> Those patches look good. Thank you. Now we just have to wait for > Parse::CPAN::Meta.
The patch is in the new Perl-Toolchain-Gang repo. I'll try to ship it tonight. David -- David Golden <xdg@xdg.me> Take back your inbox! → http://www.bunchmail.com/ Twitter/IRC: @xdg
CC: p5p <perl5-porters [...] perl.org>, niels [...] thykier.net
Subject: Re: [perl #59802] return 0 or die;
Date: Wed, 17 Jul 2013 21:46:17 -0400
To: James E Keenan via RT <perlbug-followup [...] perl.org>
From: David Golden <xdg [...] xdg.me>
Download (untitled) / with headers
text/plain 758b
Parse-CPAN-Meta-1.4405.tar.gz has entered CPAN as file: $CPAN/authors/id/D/DA/ DAGOLDEN/Parse-CPAN-Meta-1.4405.tar.gz size: 20876 bytes md5: f9025240afbd3fd98e34964609732b8b On Wed, Jul 17, 2013 at 4:59 PM, David Golden <xdg@xdg.me> wrote: Show quoted text
> On Wed, Jul 17, 2013 at 4:12 PM, Father Chrysostomos via RT > <perlbug-followup@perl.org> wrote:
>> Those patches look good. Thank you. Now we just have to wait for >> Parse::CPAN::Meta.
> > The patch is in the new Perl-Toolchain-Gang repo. > > I'll try to ship it tonight. > > David > > > -- > David Golden <xdg@xdg.me> > Take back your inbox! → http://www.bunchmail.com/ > Twitter/IRC: @xdg
-- David Golden <xdg@xdg.me> Take back your inbox! → http://www.bunchmail.com/ Twitter/IRC: @xdg
RT-Send-CC: perl5-porters [...] perl.org, xdg [...] xdg.me
Download (untitled) / with headers
text/plain 288b
On Wed Jul 17 18:47:35 2013, xdg@xdg.me wrote: Show quoted text
> Parse-CPAN-Meta-1.4405.tar.gz > > has entered CPAN as > > file: $CPAN/authors/id/D/DA/ > DAGOLDEN/Parse-CPAN-Meta-1.4405.tar.gz > size: 20876 bytes > md5: f9025240afbd3fd98e34964609732b8b
Thank you. -- Father Chrysostomos
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Fri, 19 Jul 2013 01:05:06 +0200
To: perlbug-followup [...] perl.org
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 955b
On 2013-07-17 22:12, Father Chrysostomos via RT wrote: Show quoted text
> On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote:
>> [...] >> >> Included is a patch to add the op_folded member and my previous patch >> has been rebased on top of it. >> >> RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally in the >> code (i.e. perl does the right thing even without it). However, I left >> it in (and kept it maintained) for now until B::Concise can be updated. >> I presume that will involve exposing op_folded via B.pm, which I have >> left as "future work" for now. >> I made no attempt to have it replace OPf_SPECIAL on the regexp/tr >> operators either.
> > Those patches look good. Thank you. Now we just have to wait for > Parse::CPAN::Meta. >
Revised warning patch attached with more tests. Thanks to Reini Urban for smoke testing CPAN and reminding me that exit is a unary operator (so it is not even safe to use "||" with exit). ~Niels

Message body is not shown because sender requested not to inline it.

CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>, pp <perl5-porters [...] perl.org>
Subject: Re: [perl #59802] return 0 or die;
Date: Thu, 18 Jul 2013 23:22:36 -0500
To: Niels Thykier <niels [...] thykier.net>
From: Reini Urban <rurban [...] x-ray.at>
Download (untitled) / with headers
text/plain 1.9k
On Thu, Jul 18, 2013 at 6:05 PM, Niels Thykier <niels@thykier.net> wrote: Show quoted text
> On 2013-07-17 22:12, Father Chrysostomos via RT wrote:
>> On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote:
>>> Included is a patch to add the op_folded member and my previous patch >>> has been rebased on top of it. >>> >>> RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally in the >>> code (i.e. perl does the right thing even without it). However, I left >>> it in (and kept it maintained) for now until B::Concise can be updated. >>> I presume that will involve exposing op_folded via B.pm, which I have >>> left as "future work" for now. >>> I made no attempt to have it replace OPf_SPECIAL on the regexp/tr >>> operators either.
>> >> Those patches look good. Thank you. Now we just have to wait for >> Parse::CPAN::Meta. >>
> > Revised warning patch attached with more tests. Thanks to Reini Urban > for smoke testing CPAN and reminding me that exit is a unary operator > (so it is not even safe to use "||" with exit).
LGTM. These are the bugs we found so far: Perl-Critic [cpan #87032] wrong return precedence in Perl::Critic::Utils::interpolate YAML-Syck [cpan #87034] precedence error in inc/Test/Builder.pm DBI [cpan #87029] Possible precedence issue with control flow operator at ./t/16destroy.t Git-CPAN-Patch [cpan #86950] wrong return or precedence (fixed) Apache-SWIT [cpan #86949] return expr or precedence error Parse-CPAN-Meta - fixed with 1.4405 JSON-PP (no ticket yet) Test-Simple (no ticket yet) I needed the following patch to extend B for folded (and the other missing fields), and to fix dump. And the 2nd patch is a hint for the upstream maintainers. When I fixed all B modules to use the new B, we can get rid of the OPpCONST_FOLDED and have one more private bit free for CONSTs. B::C and ByteLoader already done. B-Generate has more problems with blead. -- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/

Message body is not shown because sender requested not to inline it.

Message body is not shown because sender requested not to inline it.

CC: perlbug-followup [...] perl.org, pp <perl5-porters [...] perl.org>
Subject: Re: [perl #59802] return 0 or die;
Date: Fri, 19 Jul 2013 09:11:15 +0200
From: Niels Thykier <niels [...] thykier.net>
Download (untitled) / with headers
text/plain 1.4k
On 2013-07-19 06:22, Reini Urban wrote: Show quoted text
> On Thu, Jul 18, 2013 at 6:05 PM, Niels Thykier <niels@thykier.net> wrote:
>> [...] >> >> Revised warning patch attached with more tests. Thanks to Reini Urban >> for smoke testing CPAN and reminding me that exit is a unary operator >> (so it is not even safe to use "||" with exit).
> > LGTM. > > These are the bugs we found so far: > > Perl-Critic [cpan #87032] wrong return precedence in > Perl::Critic::Utils::interpolate > YAML-Syck [cpan #87034] precedence error in inc/Test/Builder.pm > DBI [cpan #87029] Possible precedence issue with control flow operator > at ./t/16destroy.t > Git-CPAN-Patch [cpan #86950] wrong return or precedence (fixed) > Apache-SWIT [cpan #86949] return expr or precedence error > Parse-CPAN-Meta - fixed with 1.4405 >
Great. :) Show quoted text
> JSON-PP (no ticket yet) > Test-Simple (no ticket yet) >
I believe these are filed: * https://rt.cpan.org/Public/Bug/Display.html?id=86948 * https://github.com/schwern/test-more/pull/385 Show quoted text
> I needed the following patch to extend B for folded (and the other > missing fields), > and to fix dump. > > And the 2nd patch is a hint for the upstream maintainers. > > When I fixed all B modules to use the new B, we can get rid of > the OPpCONST_FOLDED and have one more private bit free for CONSTs. > B::C and ByteLoader already done. B-Generate has more problems with blead. > > -- > Reini Urban > http://cpanel.net/ http://www.perl-compiler.org/ >
Sounds good to me. :) ~Niels
CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #59802] return 0 or die;
Date: Fri, 19 Jul 2013 10:29:20 -0500
To: pp <perl5-porters [...] perl.org>
From: Reini Urban <rurban [...] x-ray.at>
Download (untitled) / with headers
text/plain 544b
CPAN updates on this after the night run: + DBI [cpan #87029] (fixed) * Git-CPAN-Patch [cpan #86950] (fixed) * Parse-CPAN-Meta (fixed) * Perl-Critic [cpan #87032] * YAML-Syck [cpan #87034] * Apache-SWIT [cpan #86949] + JSON-PP [cpan #86948] + HTML-Mason [cpan #87050] + IO-Async [cpan #87051] + IO-Socket-SSL [cpan #87052] + Test-Simple [cpan #87053] + https://github.com/schwern/test-more/pull/385 all patches also in my distroprefs: https://github.com/rurban/distroprefs -- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/
RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 1.6k
On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote: Show quoted text
> On 2013-07-17 18:56, Father Chrysostomos via RT wrote:
> > On Wed Jul 17 01:30:56 2013, niels@thykier.net wrote:
> >> [...] > >> > >> You wrote "also set op->op_folded" and later imply the change could > >> simplify the code by using the same bit. > >> Just to confirm here, did you want the "op_folded" member replace > >> OPpCONST_FOLDED and (for regexp/tr operators) OPf_SPECIAL?
> > > > That would be nice. But it is not a prerequisite for getting your patch > > in. :-) > >
> >> And if so, > >> should OPf_SPECIAL still be set in these cases since it is a public
flag Show quoted text
> >> or should it be cleared so we can repurpose it?
> > > > I think (off the top of my head) that B::Deparse currently uses it. So > > B::Deparse may have to change. I don’t remember now. > > > > OPf_SPECIAL is a private flag, at least in terms of its use, but stored > > with the public flags. > >
> > Included is a patch to add the op_folded member and my previous patch > has been rebased on top of it. > > RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally in the > code (i.e. perl does the right thing even without it). However, I left > it in (and kept it maintained) for now until B::Concise can be updated. > I presume that will involve exposing op_folded via B.pm, which I have > left as "future work" for now. > I made no attempt to have it replace OPf_SPECIAL on the regexp/tr > operators either.
Thank you. I have applied the op_folded patch as 3513c7400. The warning patch will again have to wait, as your more thorough version of the patch triggers other warnings, causing other test failures, as I am sure you well know. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.3k
On Thu Jul 18 21:23:27 2013, rurban wrote: Show quoted text
> On Thu, Jul 18, 2013 at 6:05 PM, Niels Thykier <niels@thykier.net> wrote:
> > On 2013-07-17 22:12, Father Chrysostomos via RT wrote:
> >> On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote:
> >>> Included is a patch to add the op_folded member and my previous patch > >>> has been rebased on top of it. > >>> > >>> RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally
in the Show quoted text
> >>> code (i.e. perl does the right thing even without it). However, I
left Show quoted text
> >>> it in (and kept it maintained) for now until B::Concise can be
updated. Show quoted text
> >>> I presume that will involve exposing op_folded via B.pm, which I have > >>> left as "future work" for now. > >>> I made no attempt to have it replace OPf_SPECIAL on the regexp/tr > >>> operators either.
> >> > >> Those patches look good. Thank you. Now we just have to wait for > >> Parse::CPAN::Meta. > >>
> > > > Revised warning patch attached with more tests. Thanks to Reini Urban > > for smoke testing CPAN and reminding me that exit is a unary operator > > (so it is not even safe to use "||" with exit).
> > LGTM. > > These are the bugs we found so far: > > Perl-Critic [cpan #87032] wrong return precedence in > Perl::Critic::Utils::interpolate > YAML-Syck [cpan #87034] precedence error in inc/Test/Builder.pm > DBI [cpan #87029] Possible precedence issue with control flow operator > at ./t/16destroy.t > Git-CPAN-Patch [cpan #86950] wrong return or precedence (fixed) > Apache-SWIT [cpan #86949] return expr or precedence error > Parse-CPAN-Meta - fixed with 1.4405 > > JSON-PP (no ticket yet) > Test-Simple (no ticket yet) > > I needed the following patch to extend B for folded (and the other > missing fields), > and to fix dump. > > And the 2nd patch is a hint for the upstream maintainers. > > When I fixed all B modules to use the new B, we can get rid of > the OPpCONST_FOLDED and have one more private bit free for CONSTs. > B::C and ByteLoader already done. B-Generate has more problems with blead.
Thank you. I have applied the first patch as 3164fde474d1, but I omitted the OpFOLDED macro, because I don’t understand its purpose (or need). Niels’ patch changes the core not to use OPpCONST_FOLDED at all, so having a macro to check that doesn’t help. If it is for B::C’s sake, then it won’t be present in older perls anyway, so you will still have to #define it in B::C. -- Father Chrysostomos
CC: pp <perl5-porters [...] perl.org>
Subject: Re: [perl #59802] return 0 or die;
Date: Fri, 19 Jul 2013 13:40:51 -0500
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Reini Urban <rurban [...] x-ray.at>
Download (untitled) / with headers
text/plain 3.6k
On Fri, Jul 19, 2013 at 12:30 PM, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Thu Jul 18 21:23:27 2013, rurban wrote:
>> On Thu, Jul 18, 2013 at 6:05 PM, Niels Thykier <niels@thykier.net> wrote:
>> > On 2013-07-17 22:12, Father Chrysostomos via RT wrote:
>> >> On Wed Jul 17 12:37:07 2013, niels@thykier.net wrote:
>> >>> Included is a patch to add the op_folded member and my previous patch >> >>> has been rebased on top of it. >> >>> >> >>> RE: op_folded-patch - I made OPpCONST_FOLDED redundant internally
> in the
>> >>> code (i.e. perl does the right thing even without it). However, I
> left
>> >>> it in (and kept it maintained) for now until B::Concise can be
> updated.
>> >>> I presume that will involve exposing op_folded via B.pm, which I have >> >>> left as "future work" for now. >> >>> I made no attempt to have it replace OPf_SPECIAL on the regexp/tr >> >>> operators either.
>> >> >> >> Those patches look good. Thank you. Now we just have to wait for >> >> Parse::CPAN::Meta. >> >>
>> > >> > Revised warning patch attached with more tests. Thanks to Reini Urban >> > for smoke testing CPAN and reminding me that exit is a unary operator >> > (so it is not even safe to use "||" with exit).
>> >> LGTM. >> >> These are the bugs we found so far: >> >> Perl-Critic [cpan #87032] wrong return precedence in >> Perl::Critic::Utils::interpolate >> YAML-Syck [cpan #87034] precedence error in inc/Test/Builder.pm >> DBI [cpan #87029] Possible precedence issue with control flow operator >> at ./t/16destroy.t >> Git-CPAN-Patch [cpan #86950] wrong return or precedence (fixed) >> Apache-SWIT [cpan #86949] return expr or precedence error >> Parse-CPAN-Meta - fixed with 1.4405 >> >> JSON-PP (no ticket yet) >> Test-Simple (no ticket yet) >> >> I needed the following patch to extend B for folded (and the other >> missing fields), >> and to fix dump. >> >> And the 2nd patch is a hint for the upstream maintainers. >> >> When I fixed all B modules to use the new B, we can get rid of >> the OPpCONST_FOLDED and have one more private bit free for CONSTs. >> B::C and ByteLoader already done. B-Generate has more problems with blead.
> > Thank you. I have applied the first patch as 3164fde474d1, but I > omitted the OpFOLDED macro, because I don’t understand its purpose (or > need). Niels’ patch changes the core not to use OPpCONST_FOLDED at all, > so having a macro to check that doesn’t help. If it is for B::C’s sake, > then it won’t be present in older perls anyway, so you will still have > to #define it in B::C.
Thanks and I'm fine with that. It thought it's a cool API for the time when we have both. But I haven't found a usecase for it yet. So missing are: commit 715975678ef3c57515eb6282cdaab53da6afbb46 Author: Niels Thykier <niels@thykier.net> Date: Mon Jul 15 22:25:19 2013 +0200 op.c: Warn on "return $a or $b" [perl #59802] Add a warning for the (likely) unintended use of "return $a or $b" (and similar expressions), which perl parses as "(return $a) || $b" (which is effectively just "return $a;"). Note this warning is triggered by some modules (e.g. Test::Builder). These are not fixed by this commit. Signed-off-by: Niels Thykier <niels@thykier.net> commit c1f7aff8b732c59411569240ce4ffc7a83080e33 Author: Reini Urban <rurban@x-ray.at> Date: Thu Jul 11 12:09:15 2013 -0500 Return B::HEK for B::CV::GV of lexical subs A lexsub has a hek instead of a gv. Provide a ref to a PV for the name in the new B::HEK class. This crashed previously accessing the not existing SvFLAGS of the hek. -- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 22 Jul 2013 17:59:09 -0400
To: Niels Thykier <niels [...] thykier.net>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 292b
* Niels Thykier <niels@thykier.net> [2013-07-16T04:58:01] Show quoted text
> Here is a revised patch. > […] > The warning is now emitted for any of the following ops: > OP_{NEXT,LAST,REDO,RETURN,DIE,EXIT,GOTO}
Thanks, Niels, this warning is something I'm very happy to see finally happening! -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 710b
On Mon Jul 22 14:59:51 2013, perl.p5p@rjbs.manxome.org wrote: Show quoted text
> * Niels Thykier <niels@thykier.net> [2013-07-16T04:58:01]
> > Here is a revised patch. > > […] > > The warning is now emitted for any of the following ops: > > OP_{NEXT,LAST,REDO,RETURN,DIE,EXIT,GOTO}
> > Thanks, Niels, this warning is something I'm very happy to see finally > happening!
This is currently blocked by Test::Builder. If we are to have this warning, we probably ought to have it present in bleadperl for as long as possible, in order to shake out CPAN bugs. Would it be acceptable to break the usual rule and patch Test::Builder directly in blead (assigning it a previously unused dev version)? -- Father Chrysostomos
CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Thu, 22 Aug 2013 22:17:24 -0400
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 480b
* Father Chrysostomos via RT <perlbug-followup@perl.org> [2013-08-20T11:51:46] Show quoted text
> Would it be acceptable to break the usual rule and patch Test::Builder > directly in blead (assigning it a previously unused dev version)?
I've just poked Schwern both via GitHub and another channel. Let's give him a day or two to state his intentions before patching blead... which we should do if needed. (and if you do that, please add a ticket to #116923 which I will follow up on) -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

CC: perl5-porters [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 26 Aug 2013 00:28:09 -0400
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 378b
* Ricardo Signes <perl.p5p@rjbs.manxome.org> [2013-08-22T22:17:24] Show quoted text
> I've just poked Schwern both via GitHub and another channel. Let's give him > a day or two to state his intentions before patching blead... which we should > do if needed. (and if you do that, please add a ticket to #116923 which I > will follow up on)
FWIW, Schwern is now AFK for a week or so. -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 523b
On Sun Aug 25 21:28:46 2013, perl.p5p@rjbs.manxome.org wrote: Show quoted text
> * Ricardo Signes <perl.p5p@rjbs.manxome.org> [2013-08-22T22:17:24]
> > I've just poked Schwern both via GitHub and another channel. Let's
> give him
> > a day or two to state his intentions before patching blead... which
> we should
> > do if needed. (and if you do that, please add a ticket to #116923
> which I
> > will follow up on)
> > FWIW, Schwern is now AFK for a week or so.
What does AFK stand for? Asking for kippers? -- Father Chrysostomos
CC: perlbug-followup [...] perl.org
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 26 Aug 2013 09:41:31 +0200
To: perl5-porters [...] perl.org
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Download (untitled) / with headers
text/plain 990b
On Mon, 26 Aug 2013 00:09:12 -0700, "Father Chrysostomos via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> On Sun Aug 25 21:28:46 2013, perl.p5p@rjbs.manxome.org wrote:
> > * Ricardo Signes <perl.p5p@rjbs.manxome.org> [2013-08-22T22:17:24]
> > > I've just poked Schwern both via GitHub and another channel. Let's
> > give him
> > > a day or two to state his intentions before patching blead... which
> > we should
> > > do if needed. (and if you do that, please add a ticket to #116923
> > which I
> > > will follow up on)
> > > > FWIW, Schwern is now AFK for a week or so.
> > What does AFK stand for? Asking for kippers?
Away From Keyboard http://www.acronymfinder.com/Away-From-Keyboard-(AFK).html -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
CC: pp <perl5-porters [...] perl.org>
Subject: Re: [perl #59802] return 0 or die;
Date: Mon, 26 Aug 2013 15:02:17 -0500
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
From: Reini Urban <rurban [...] x-ray.at>
Download (untitled) / with headers
text/plain 1.2k
On Tue, Aug 20, 2013 at 10:51 AM, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Mon Jul 22 14:59:51 2013, perl.p5p@rjbs.manxome.org wrote:
>> * Niels Thykier <niels@thykier.net> [2013-07-16T04:58:01]
>> > Here is a revised patch. >> > […] >> > The warning is now emitted for any of the following ops: >> > OP_{NEXT,LAST,REDO,RETURN,DIE,EXIT,GOTO}
>> >> Thanks, Niels, this warning is something I'm very happy to see finally >> happening!
> > This is currently blocked by Test::Builder. If we are to have this > warning, we probably ought to have it present in bleadperl for as long > as possible, in order to shake out CPAN bugs.
All possible CPAN bugs have already been reported by smoking CPAN with a patched bleadperl with these warning enabled last month. The fastest response was from DBI, the slowest seems to be Test::Builder. Now only BlackPAN is the problem. Our own company blackpan was okay. See http://blogs.perl.org/users/rurban/2013/07/smoking-cpan.html All necessary distroprefs patches are in my repo https://github.com/rurban/distroprefs Show quoted text
> Would it be acceptable to break the usual rule and patch Test::Builder > directly in blead (assigning it a previously unused dev version)?
-- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/
RT-Send-CC: perl5-porters [...] perl.org, niels [...] thykier.net
Download (untitled) / with headers
text/plain 757b
On Thu Aug 22 19:17:59 2013, perl.p5p@rjbs.manxome.org wrote: Show quoted text
> * Father Chrysostomos via RT <perlbug-followup@perl.org> [2013-08- > 20T11:51:46]
> > Would it be acceptable to break the usual rule and patch
> Test::Builder
> > directly in blead (assigning it a previously unused dev version)?
> > I've just poked Schwern both via GitHub and another channel. Let's > give him a > day or two to state his intentions before patching blead... which we > should do > if needed. (and if you do that, please add a ticket to #116923 which > I will > follow up on)
I have applied Niels’ warning patch now as 9da2d0467dc5 (thank you). His precedence patch has been applied as 13c65ef8cd5. I have opened ticket #119825 to track this. -- Father Chrysostomos


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