Mailing List Archive

[perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz
# New Ticket Created by (Andreas J. Koenig)
# Please include the string: [perl #113684]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=113684 >


git bisect
----------
6a31dbf44ee919c340a3372c95b28d581979d165 is the first bad commit
commit 6a31dbf44ee919c340a3372c95b28d581979d165
Author: Father Chrysostomos <sprout@cpan.org>
Date: Fri Jun 8 14:49:57 2012 -0700

B::Deparse: loopexes have list prec


sample fail report
------------------
http://www.cpantesters.org/cpan/report/9241afd6-b6d9-11e1-9c05-a8133af89482

perl -V
-------
Summary of my perl5 (revision 5 version 17 subversion 0) configuration:
Commit id: 6a31dbf44ee919c340a3372c95b28d581979d165
Platform:
osname=linux, osvers=3.2.0-2-amd64, archname=x86_64-linux-ld
uname='linux k83 3.2.0-2-amd64 #1 smp mon may 21 17:45:41 utc 2012 x86_64 gnulinux '
config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.17.0-326-g6a31dbf/127e -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Uuseithreads -Duselongdouble -DDEBUGGING=-g'
hint=recommended, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=define, uselongdouble=define
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2 -g',
cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
ccversion='', gccversion='4.7.0', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t', lseeksize=8
alignbytes=16, prototype=define
Linker and Libraries:
ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
libc=, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.13'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'


Characteristics of this binary (from libperl):
Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV
PERL_MALLOC_WRAP PERL_PRESERVE_IVUV PERL_USE_DEVEL
USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES
USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE
USE_LOCALE_NUMERIC USE_LONG_DOUBLE USE_PERLIO
USE_PERL_ATOF
Built under linux
Compiled at Jun 16 2012 19:52:34
@INC:
/home/src/perl/repoperls/installed-perls/perl/v5.17.0-326-g6a31dbf/127e/lib/site_perl/5.17.0/x86_64-linux-ld
/home/src/perl/repoperls/installed-perls/perl/v5.17.0-326-g6a31dbf/127e/lib/site_perl/5.17.0
/home/src/perl/repoperls/installed-perls/perl/v5.17.0-326-g6a31dbf/127e/lib/5.17.0/x86_64-linux-ld
/home/src/perl/repoperls/installed-perls/perl/v5.17.0-326-g6a31dbf/127e/lib/5.17.0
.

--
andreas
[perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Sat Jun 16 14:57:33 2012, paul@pjcj.net wrote:
> On Sat, Jun 16, 2012 at 08:39:06PM +0200, Andreas J. Koenig wrote:
> > git bisect
> > ----------
> > 6a31dbf44ee919c340a3372c95b28d581979d165 is the first bad commit
> > commit 6a31dbf44ee919c340a3372c95b28d581979d165
> > Author: Father Chrysostomos <sprout@cpan.org>
> > Date: Fri Jun 8 14:49:57 2012 -0700
> >
> > B::Deparse: loopexes have list prec
>
> Thanks again for BBC. I've been getting cpantesters failures from
> 5.17.0, though an official 5.17.0 works fine for Devel::Cover, so I
> guessed there was more recent commit involved.
>
> We've been talking about bumping version numbers before and after
> releases. Doing so would make make this more obvious and is something I
> would support.
>
> This commit is good. It removes some superfluous parentheses (not many
> and only a little bit annoying) which were new in 5.16.0. I'll fix up
> Devel::Cover for 5.17.1, so nothing more needs to be done to perl here.
> I'll try to remember to send a further reply when blead and Devel::Cover
> are happy with each other again.

Actually, you might want to wait. I’m thinking of reverting that
commit, because I realise it is wrong.

I noticed through experimentation that loop exits (last, next) have low
precedence. What I didn’t realise is that they have their own
precedence level, unlisted in perlop:

diff --git a/pod/perlop.pod b/pod/perlop.pod
index 3edeabd..c9a1adf 100644
--- a/pod/perlop.pod
+++ b/pod/perlop.pod
@@ -49,6 +49,7 @@ values only, not array values.
nonassoc .. ...
right ?:
right = += -= *= etc.
+ nonassoc loop exits (last, next, goto)
left , =>
nonassoc list operators (rightward)
right not

That could be considered a bug in perl. In any case, I’d just muddied
things a bit by trying to fix Deparse without realising what perl was
actually doing.

Changing the precedence of goto might break it, as it takes an arbitrary
expression. Other ‘loopexes’ are simply buggy when it comes to
arbitrary expressions, treating anything that is not a simple constant
as the empty label "". But I think they should match goto (not only in
precedence, but in allowing computed labels).

If it’s OK with you, I’ll revert the blead change for now, until we
figure out what it is that needs fixing.

--

Father Chrysostomos


---
via perlbug: queue: perl5 status: new
https://rt.perl.org:443/rt3/Ticket/Display.html?id=113684
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Sat, Jun 16, 2012 at 06:49:50PM -0700, Father Chrysostomos via RT wrote:
> On Sat Jun 16 14:57:33 2012, paul@pjcj.net wrote:
> > On Sat, Jun 16, 2012 at 08:39:06PM +0200, Andreas J. Koenig wrote:
> > > git bisect
> > > ----------
> > > 6a31dbf44ee919c340a3372c95b28d581979d165 is the first bad commit
> > > commit 6a31dbf44ee919c340a3372c95b28d581979d165
> > > Author: Father Chrysostomos <sprout@cpan.org>
> > > Date: Fri Jun 8 14:49:57 2012 -0700
> > >
> > > B::Deparse: loopexes have list prec

> Actually, you might want to wait. I’m thinking of reverting that
> commit, because I realise it is wrong.

> If it’s OK with you, I’ll revert the blead change for now, until we
> figure out what it is that needs fixing.

That's fine by me. Devel::Cover will pass its tests again, and when you
are able commit something you are happy with I will update Devel::Cover.
I'm always happy to do that for B::Deparse improvements.

Thanks very much for caring about this.

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
[perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Sun Jun 17 09:12:19 2012, paul@pjcj.net wrote:
> On Sat, Jun 16, 2012 at 06:49:50PM -0700, Father Chrysostomos via RT
wrote:
> > On Sat Jun 16 14:57:33 2012, paul@pjcj.net wrote:
> > > On Sat, Jun 16, 2012 at 08:39:06PM +0200, Andreas J. Koenig wrote:
> > > > git bisect
> > > > ----------
> > > > 6a31dbf44ee919c340a3372c95b28d581979d165 is the first bad commit
> > > > commit 6a31dbf44ee919c340a3372c95b28d581979d165
> > > > Author: Father Chrysostomos <sprout@cpan.org>
> > > > Date: Fri Jun 8 14:49:57 2012 -0700
> > > >
> > > > B::Deparse: loopexes have list prec
>
> > Actually, you might want to wait. I’m thinking of reverting that
> > commit, because I realise it is wrong.
>
> > If it’s OK with you, I’ll revert the blead change for now, until we
> > figure out what it is that needs fixing.
>
> That's fine by me. Devel::Cover will pass its tests again, and when you
> are able commit something you are happy with I will update Devel::Cover.
> I'm always happy to do that for B::Deparse improvements.

I have reverted it as 23e485c842f3b.

--

Father Chrysostomos


---
via perlbug: queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=113684
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Fri, Jul 13, 2012 at 08:16:47PM -0700, Father Chrysostomos via RT wrote:
> On Sat Jun 16 18:49:49 2012, sprout wrote:
> > I noticed through experimentation that loop exits (last, next) have low
> > precedence. What I didn’t realise is that they have their own
> > precedence level, unlisted in perlop:
> >
> > diff --git a/pod/perlop.pod b/pod/perlop.pod
> > index 3edeabd..c9a1adf 100644
> > --- a/pod/perlop.pod
> > +++ b/pod/perlop.pod
> > @@ -49,6 +49,7 @@ values only, not array values.
> > nonassoc .. ...
> > right ?:
> > right = += -= *= etc.
> > + nonassoc loop exits (last, next, goto)
> > left , =>
> > nonassoc list operators (rightward)
> > right not
> >
> > That could be considered a bug in perl. In any case, I’d just muddied
> > things a bit by trying to fix Deparse without realising what perl was
> > actually doing.
> >
> > Changing the precedence of goto might break it, as it takes an arbitrary
> > expression. Other ‘loopexes’ are simply buggy when it comes to
> > arbitrary expressions, treating anything that is not a simple constant
> > as the empty label "". But I think they should match goto (not only in
> > precedence, but in allowing computed labels).
>
> Can we try to decide how this should work? Should goto’s real
> precedence be listed in perlop.pod? Or should we fix its precedence?
> If the latter, which precedence shall it have?

I'm probably not the right person to be opining on this, but I didn't
want the thread to get lost, so here's my tuppence ha'penny worth.

I can't see any reason why goto should have a different precedence level
to last, next and so forth. Is there one? And in terms of elegance and
aesthetics I think a shorter precedence table is preferable to a longer
one.

So should goto's precedence be changed to that of the other loop exits,
or the other way around? You seem to be leaning towards the latter,
which would ensure there were no problems with C<goto expr>. But I
suspect the other loop exits are more common.

You don't say where goto sits in the precedence table at the moment, but
I'm assuming it's pretty close to the other loop exits. Why do you
think they should have the same precedence as goto? Lacking your
understanding, I'd imagine it should be the other way around.

However, I'd also imagine any breakage from such changing of precedence
(either way) to be minimal. So once again we come to the question of
whether the cost in backwards compatibility is worth the benefit. In
this case I would suspect the benefits of a simpler language to outweigh
the backwards compatibility cost, but this is more of a gut feeling than
an opinion backed by any evidence.

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
* Father Chrysostomos via RT <perlbug-followup@perl.org> [2012-07-13T23:16:47]
> Can we try to decide how this should work? Should goto’s real
> precedence be listed in perlop.pod? Or should we fix its precedence?
> If the latter, which precedence shall it have?

I'm just back from a week away, but will give this some real thought soon.

--
rjbs
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
* Father Chrysostomos via RT <perlbug-followup@perl.org> [2012-07-13T23:16:47]
> Can we try to decide how this should work? Should goto’s real
> precedence be listed in perlop.pod? Or should we fix its precedence?
> If the latter, which precedence shall it have?

So, I've read over this whole ticket thread a few times, and I think that
either I am confused or Paul is confused.

It seems to me that the current issue is twofold:

1. loop exits have a distinct precedence not in perlop's table
(and this precedence is the same for goto/last/next/redo)

2. non-goto exits do not understand non-constant targets

Paul seems to have understood that goto and last/next have precedences distinct
from one another.

Either Paul is mistaken or I am mistaken, or I am mistaken about what Paul
thinks.

At any rate, if I am not the mistaken one, then I think that we should document
the existing precedence and, if possible, see whether fixing the labels allowed
to loop exits will Break Everything. If not, it would be nice to have.

--
rjbs
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Thu, Jul 26, 2012 at 10:47:09PM -0400, Ricardo Signes wrote:
> * Father Chrysostomos via RT <perlbug-followup@perl.org> [2012-07-13T23:16:47]
> > Can we try to decide how this should work? Should goto’s real
> > precedence be listed in perlop.pod? Or should we fix its precedence?
> > If the latter, which precedence shall it have?
>
> So, I've read over this whole ticket thread a few times, and I think that
> either I am confused or Paul is confused.

> Paul seems to have understood that goto and last/next have precedences distinct
> from one another.

Yes, that's the impression I had got from the thread.

> Either Paul is mistaken or I am mistaken, or I am mistaken about what Paul
> thinks.

I'm pretty sure it's me being mistaken, which is a better state of
affairs for everyone else.

> At any rate, if I am not the mistaken one, then I think that we should document
> the existing precedence and, if possible, see whether fixing the labels allowed
> to loop exits will Break Everything. If not, it would be nice to have.

I think this is where I was suggesting we end up, but I had us starting
from the wrong place. So apologies for simply muddying the waters. As
soon as Father Chrysostomos is happy with the core I will make whatever
changes are necessary to bring Devel::Cover into line with it.

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Fri, Jul 27, 2012 at 04:29:54PM -0700, Father Chrysostomos via RT wrote:

> In commit 1f039d60d3, I have fixed loop exits to accept computed labels,
> documented it in 8a7e748e29.
>
> In commit 1eb0b7be2f, I have made B::Deparse deparse them with the
> correct precedence.

Thanks for doing this.

So now we have:

$ perl -MO=Deparse -e '$a = $b || next'
$a = $b || (next);

Those parentheses are new (from 5.16.0) and superfluous. Are they there
by design?

For reference:

$ perl -MO=Deparse -e '$a = $b || return'
$a = $b || (return);

Those parentheses first appeared in 5.14.0.

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Thu, Aug 02, 2012 at 12:46:33PM -0700, Father Chrysostomos via RT wrote:
> On Thu Aug 02 10:54:27 2012, paul@pjcj.net wrote:
> > On Fri, Jul 27, 2012 at 04:29:54PM -0700, Father Chrysostomos via RT
> wrote:
> >
> > > In commit 1f039d60d3, I have fixed loop exits to accept computed labels,
> > > documented it in 8a7e748e29.
> > >
> > > In commit 1eb0b7be2f, I have made B::Deparse deparse them with the
> > > correct precedence.
> >
> > Thanks for doing this.
> >
> > So now we have:
> >
> > $ perl -MO=Deparse -e '$a = $b || next'
> > $a = $b || (next);
> >
> > Those parentheses are new (from 5.16.0) and superfluous. Are they there
> > by design?
> >
> > For reference:
> >
> > $ perl -MO=Deparse -e '$a = $b || return'
> > $a = $b || (return);
> >
> > Those parentheses first appeared in 5.14.0.
>
> Please see my explanation at
> <https://rt.perl.org/rt3/Ticket/Display.html?id=106892#txn-1043534>.
> The same applies here. I don’t want to rewrite B::Deparse completely
> just yet. :-)

Thanks for the explanation (again). As soon as 5.17.3 comes out I'll
fix up Devel::Cover to take this into account.

Thanks again,

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net
Re: [perl #113684] Bleadperl v5.17.0-326-g6a31dbf breaks PJCJ/Devel-Cover-0.89.tar.gz [ In reply to ]
On Thu, Aug 02, 2012 at 09:57:45PM +0200, Paul Johnson wrote:
> On Thu, Aug 02, 2012 at 12:46:33PM -0700, Father Chrysostomos via RT wrote:
> > On Thu Aug 02 10:54:27 2012, paul@pjcj.net wrote:
> > > On Fri, Jul 27, 2012 at 04:29:54PM -0700, Father Chrysostomos via RT
> > wrote:
> > >
> > > > In commit 1f039d60d3, I have fixed loop exits to accept computed labels,
> > > > documented it in 8a7e748e29.
> > > >
> > > > In commit 1eb0b7be2f, I have made B::Deparse deparse them with the
> > > > correct precedence.
> > >
> > > Thanks for doing this.
> > >
> > > So now we have:
> > >
> > > $ perl -MO=Deparse -e '$a = $b || next'
> > > $a = $b || (next);
> > >
> > > Those parentheses are new (from 5.16.0) and superfluous. Are they there
> > > by design?
> > >
> > > For reference:
> > >
> > > $ perl -MO=Deparse -e '$a = $b || return'
> > > $a = $b || (return);
> > >
> > > Those parentheses first appeared in 5.14.0.
> >
> > Please see my explanation at
> > <https://rt.perl.org/rt3/Ticket/Display.html?id=106892#txn-1043534>.
> > The same applies here. I don’t want to rewrite B::Deparse completely
> > just yet. :-)
>
> Thanks for the explanation (again). As soon as 5.17.3 comes out I'll
> fix up Devel::Cover to take this into account.
>
> Thanks again,

Just to close this out, this is fixed in Devel::Cover 0.94 with 5687c8d.

(I *knew* I shouldn't have written "As soon as".)

--
Paul Johnson - paul@pjcj.net
http://www.pjcj.net