Front page | perl.perl5.porters |
Postings from January 2012
Re: defined(@arr), defined (%hash)
Thread Previous
|
Thread Next
From:
Father Chrysostomos
Date:
January 22, 2012 12:31
Subject:
Re: defined(@arr), defined (%hash)
Message ID:
E7D18EA1-FDDD-4610-B737-107819795D19@cpan.org
Matthew Horsfall wrote:
> On Tue, Jan 17, 2012 at 10:53 AM, Nicholas Clark <nick@ccl4.org> wrote:
>
> > On Sun, Jan 15, 2012 at 02:18:43PM -0500, Matthew Horsfall (alh) wrote:
> > > If my method for getting the lhs of an OP_AASSIGN is correct, I think
> > I'll
> > > make a macro of it and fix a few other related outstanding bugs using
> > that
> > > new macro.
> >
> > I'm really not familiar with the structure of the optree, so I'm not
> > actually the right person to confirm this. I can look at it and say
> > "it sure looks like it works", and even compare it to some "typical"
> > B::Concise output, but that's not actually useful. I don't know the
> > "unknown unknowns" here - whether there are any corner cases that it
> > misses.
> >
> >
> > git's diff isn't being totally helpful here, mixing up all the +s and -s
> >
> > Your new code is:
> >
> > if (ckWARN_d(WARN_DEPRECATED)) {
> > register OP *kid = o->op_flags & OPf_KIDS ? cUNOPo->op_first : NULL;
> >
> > if (kid) {
> > OPCODE type = kid->op_type;
> >
> > /* OP_AASSIGN? Walk the op tree to get what's being assigned to
> > */
> > if (type == OP_AASSIGN) {
> > type =
> > cUNOPx(cBINOPx(kid)->op_last)->op_first->op_sibling->op_type;
> > }
> >
> > switch (type) {
> > case OP_RV2AV:
> > case OP_PADAV:
> > Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
> > "defined(@array) is deprecated");
> > Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
> > "\t(Maybe you should just omit the
> > defined()?)\n");
> > break;
> > case OP_RV2HV:
> > case OP_PADHV:
> > Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
> > "defined(%%hash) is deprecated");
> > Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
> > "\t(Maybe you should just omit the
> > defined()?)\n");
> > break;
> > default:
> > /* no warning */
> > break;
> > }
> > }
> > }
> >
> >
> > *I* simply don't know if that is sufficiently correct.
> > I am not the droids you're looking for.
> >
>
> Nicholas,
>
> 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 :)
>
> 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.
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).
Thread Previous
|
Thread Next