Re: defined(@arr), defined (%hash)

Front page | perl.perl5.porters | Postings from January 2012

Re: defined(@arr), defined (%hash)

Thread Previous | Thread Next

From:

Nicholas Clark

Date:

January 25, 2012 09:35

Subject:

Re: defined(@arr), defined (%hash)

Message ID:

20120125173458.GS9069@plum.flirble.org

On Sun, Jan 22, 2012 at 12:31:20PM -0800, Father Chrysostomos wrote:
> Matthew Horsfall wrote:

> > I've had a few others look at it and most gave that same response. It's not
> > pretty, but it successfully traverses the op tree, and doesn't break any
> > tests :)

You'd be surprised by how many obscure things one can change that turn out
not to have tests.

> > I think it's good to go in until proven otherwise.
> 
> I understand the patch, and can see, even without checking, that it
> bypasses the pushmark on the lhs of aassign and looks at the next op,
> which is safe even in the case of an empty list, as that consists of a
> stub op.

That's what I didn't know, and so couldn't say with confidence.
That there's always going to be something there, so it will never be a
NULL pointer.

> I don't think the patch is appropriate, as it's defined(@array) and
> defined(%hash) that are being deprecated, not defined(@array=...).  An
> aggregate on the lhs of a list assignment is not touched by the list
> assignment's parent op, so it's unrelated.  Also (without actually trying
> it) the patch will warn for defined((@a,$a) = stuff), but not for
> defined(($a,@a) = stuff).

without:

$ ./perl -le 'print defined (($a, %a) = (1,2,3))'
defined(@array) is deprecated at -e line 1.
        (Maybe you should just omit the defined()?)
1
$ ./perl -le 'print defined ((%a, $a) = (1,2,3))'
defined(@array) is deprecated at -e line 1.
        (Maybe you should just omit the defined()?)
1

with:

$ ./perl -le 'print defined (($a, %a) = (1,2,3))'
1
$ ./perl -le 'print defined ((%a, $a) = (1,2,3))'
defined(%hash) is deprecated at -e line 1.
        (Maybe you should just omit the defined()?)
1


and aaargh, yes, this is starting to seem like the same thing that caused
bugs with the in-place sort optimisation - that the LHS of a list
assignment is a list, even if it just looks like an array.

So you're starting to make me doubt whether the patch is correct, because
I'm starting to wonder whether the warning is correct for assignment,
given that what's actually being tested isn't the array (or hash), but
the list in scalar context:

Starting program: /Volumes/Stuff/Perl/perl/perl -le print\ defined\ \(\(%a,\ \$a\)\ =\ \(1,2,3\)\)
Reading symbols for shared libraries ++.. done
defined(@array) is deprecated at -e line 1.
        (Maybe you should just omit the defined()?)

Breakpoint 1, Perl_pp_defined () at pp_hot.c:397
397         dVAR; dSP;
(gdb) n
400         const int op_type = PL_op->op_type;
(gdb) 
401         const bool is_dor = (op_type == OP_DOR || op_type == OP_DORASSIGN);
(gdb) 
403         if (is_dor) {
(gdb) 
414             sv = POPs;
(gdb) 
415             if (!sv || !SvANY(sv))
(gdb) 
419         defined = FALSE;
(gdb) 
420         switch (SvTYPE(sv)) {
(gdb) 
434             SvGETMAGIC(sv);
(gdb) call Perl_sv_dump(sv)
SV = IV(0x100812a60) at 0x100812a70
  REFCNT = 1
  FLAGS = (PADTMP,IOK,pIOK)
  IV = 3


(which would clearly mean that the current code is also incorrect - the
case for OP_AASSIGN should be dropped, and let default handle it:

	default:
	    /* no warning */
	    break;

)

Nicholas Clark

Thread Previous | Thread Next