develooper Front page | perl.perl5.porters | Postings from October 2005

Re: oddity in op.c

Thread Previous | Thread Next
From:
Marcus Holland-Moritz
Date:
October 2, 2005 09:52
Subject:
Re: oddity in op.c
Message ID:
20051002185230.0fdd879b@r2d2
On 2005-09-20, at 21:33:49 +0200, H.Merijn Brand wrote:

> On Tue, 20 Sep 2005 11:21:40 -0700, Yitzchak Scott-Thoennes
> <sthoenna@efn.org> wrote:
> 
> > On Mon, Sep 19, 2005 at 11:31:19PM +0100, Nicholas Clark wrote:
> > > Merijn notices this line in 5.8.x in S_new_logop
> > > 
> > >   if ((type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv))) {
> > > 
> > > It's wonky. It seems to be wrong. But it's been that way since 5.8.0
> > > started.
> > > 
> > > The corresponding point in blead is
> > > 
> > >   if ((type == OP_AND &&  SvTRUE(((SVOP*)first)->op_sv)) ||
> > >       (type == OP_OR  && !SvTRUE(((SVOP*)first)->op_sv)) ||
> > >       (type == OP_DOR && !SvOK(((SVOP*)first)->op_sv))) {
> > > 
> > > 
> > > This was patched by Marcus Holland-Moritz, changing that == to && in the 
> > > process:
> > > 
> > > http://public.activestate.com/cgi-bin/perlbrowse?patch=22625
> > > 
> > > Presumably '==' is wrong. But if this is a bug, how can it be detected
> > > with a perl level regression test?
> > 
> > == is correct; if both operands are 0 or 1,  a == b is equivalent
> > to !(a ^ b) is equivalent to !(a && !b || !a && b) is equivlent to
> > a && b || !a && !b.
> 
> I just didn't understand why it was changed.

It was changed to fix a bug :-)

The problem was not that

   if ((type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv))) {

was wrong it itself, but that it only handled OP_AND and
(not OP_AND) aka OP_OR, which was ok when there was no OP_DOR.

But with OP_DOR, (not OP_AND) could be OP_OR or OP_DOR, so this
couldn't be handled correctly with the == expression above.

Ok, I guess I could have changed it to:

   if ((type == OP_DOR ? !SvOK(((SVOP*)first)->op_sv)) :
       (type == OP_AND) == (SvTRUE(((SVOP*)first)->op_sv)))

However, I'm not sure if that's more readable.

Marcus

-- 
Leela: "We've blown out one of our engines." 
Fry: "Fix it, fix it, fix it, fix it, fix it, fix it... fix it, fix it, fix it!"

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About