Fix bug in inlining some CORE::* subs
authorAaron Crane <arc@cpan.org>
Tue, 15 Jul 2014 17:28:42 +0000 (18:28 +0100)
committerAaron Crane <arc@cpan.org>
Fri, 1 Aug 2014 15:30:45 +0000 (16:30 +0100)
As of 4aaa475724fbbc4ab2427743fa4d07a12e6ce0d9, when Perl compiles code like

    BEGIN { *entangle = \&CORE::tie }
    entangle $foo, $package

the apparent call to "entangle" is replaced with an inlined invocation of
the "tie" builtin.

For unary ops, the OPf_SPECIAL flag was set on the generated inlined op iff
the op's argument was surrounded by parens. But that's incorrect for ops
which have their own interpretation of OPf_SPECIAL. In particular:

  keys, values, each
    OPf_SPECIAL is set for lvalue usage; this shows up when the compile-time
    argument is a reference to be subjected to the autoderef feature, and
    the run-time argument is an array ref. The existing tests didn't execute
    the code (and didn't combine inlining with array autoderefs), so didn't
    catch this case.

  delete, exists
    OPf_SPECIAL is set when the argument is an array element rather than a
    hash element; this doesn't directly cause any obvious problems, because
    pp_delete and pp_exists consider OPf_SPECIAL only when they've already
    determined that their argument is an array element, but it did break
    deparsing (because B::Deparse considers OPf_SPECIAL in all cases): the
    hash element argument got deparsed as an array element. Further, the
    inlining tests themselves rely on deparsing to ensure that the op was
    inlined. The existing inlining tests happened to use array elements, so
    didn't catch this problem.

This commit fixes those cases, by avoiding setting OPf_SPECIAL when inlining
an invocation of one of those ops. The list of op types is hard-coded; this
seems a little icky, but I don't see a better alternative.

I believe that no other ops are affected by this issue, but my confidence in
that statement isn't as high as it might be; it seems hard to work out
exactly which ops use OPf_SPECIAL.

op.c
t/op/coresubs.t

index 9ae3392..7bdfbce 100644 (file)
--- a/op.c
+++ b/op.c
@@ -10485,7 +10485,7 @@ Perl_ck_entersub_args_core(pTHX_ OP *entersubop, GV *namegv, SV *protosv)
     }
     else {
        OP *prev, *cvop, *first, *parent;
-       U32 flags;
+       U32 flags = 0;
 
         parent = entersubop;
        if (!OP_HAS_SIBLING(aop)) {
@@ -10500,7 +10500,12 @@ Perl_ck_entersub_args_core(pTHX_ OP *entersubop, GV *namegv, SV *protosv)
             OP_HAS_SIBLING(cvop);
             prev = cvop, cvop = OP_SIBLING(cvop))
            ;
-       flags = OPf_SPECIAL * !(cvop->op_private & OPpENTERSUB_NOPAREN);
+        if (!(cvop->op_private & OPpENTERSUB_NOPAREN)
+            /* Usually, OPf_SPECIAL on a UNOP means that its arg had no
+             * parens, but these have their own meaning for that flag: */
+            && opnum != OP_VALUES && opnum != OP_KEYS && opnum != OP_EACH
+            && opnum != OP_DELETE && opnum != OP_EXISTS)
+                flags |= OPf_SPECIAL;
         /* excise cvop from end of sibling chain */
         op_sibling_splice(parent, prev, 1, NULL);
        op_free(cvop);
index 4443e10..58f7d5f 100644 (file)
@@ -156,6 +156,21 @@ $tests++;
 ok eval { *CORE::exit = \42 },
   '[rt.cpan.org #74289] *CORE::foo is not accidentally made read-only';
 
+for my $word (qw<keys values each>) {
+    # mykeys() etc were aliased to \&CORE::keys etc above
+    my $code = qq{
+        no warnings 'experimental::autoderef';
+        my \$x = [];
+        () = my$word(\$x);
+        'ok'
+    };
+    $tests++;
+    is(eval($code), 'ok', "inlined $word() on autoderef array") or diag $@;
+}
+
+inlinable_ok($_, '$_{k}', 'on hash')
+    for qw<delete exists>;
+
 @UNIVERSAL::ISA = CORE;
 is "just another "->ucfirst . "perl hacker,\n"->ucfirst,
    "Just another Perl hacker,\n", 'coresubs do not return TARG';