Fix several array-returning bugs in lvalue subs

This finishes fixing bug #23790.

When called in reference context (for(...) or map $_, ...), lvalue
subs returning arrays or hashes would return the AV or HV itself, as
though it were lvalue context.

The result was that $_ would be bound to an AV or HV, which is not
supposed to happen, as it’s a scalar (that’s when you start getting
‘Bizarre copy’ errors).

Commit 91e34d82 fixed this in pp_leavesublv, but the if condition it
added was placed outside the loop, so it only applied when the array
was the first thing returned. It also did not take hashes into account.

By changing the lvalue-context check in pp_padav, pp_padhv and
pp_rv2av (which also serves as pp_rv2hv), I was able to apply a more
general fix, which also fix another bug: Those array and hash ops were
croaking when called in scalar reference context (...->$method).

Because it is no longer part of the sub-leaving code, explicitly
returning an array in reference context works now, too.

This commit also eliminates the code added by 91e34d82, as it’s no
longer necessary.

1 files changed, 8 insertions, 24 deletions

diff --git a/pp_hot.c b/pp_hot.c
index 7d0c6ec048..34c493baab 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -831,11 +831,14 @@ PP(pp_rv2av)
 	    SETs(sv);
 	    RETURN;
 	}
-	else if (LVRET) {
+	else if (PL_op->op_private & OPpMAYBE_LVSUB) {
+	  const I32 flags = is_lvalue_sub();
+	  if (flags && !(flags & OPpENTERSUB_INARGS)) {
 	    if (gimme != G_ARRAY)
 		goto croak_cant_return;
 	    SETs(sv);
 	    RETURN;
+	  }
 	}
 	else if (PL_op->op_flags & OPf_MOD
 		&& PL_op->op_private & OPpLVAL_INTRO)
@@ -873,11 +876,14 @@ PP(pp_rv2av)
 		SETs(sv);
 		RETURN;
 	    }
-	    else if (LVRET) {
+	    else if (PL_op->op_private & OPpMAYBE_LVSUB) {
+	      const I32 flags = is_lvalue_sub();
+	      if (flags && !(flags & OPpENTERSUB_INARGS)) {
 		if (gimme != G_ARRAY)
 		    goto croak_cant_return;
 		SETs(sv);
 		RETURN;
+	      }
 	    }
 	}
     }
@@ -2685,28 +2691,6 @@ PP(pp_leavesublv)
 	    goto rvalue;
 	if (gimme == G_ARRAY) {
 	    mark = newsp + 1;
-	    /* We want an array here, but padav will have left us an arrayref for an lvalue,
-	     * so we need to expand it */
-	    if(SvTYPE(*mark) == SVt_PVAV) {
-		AV *const av = MUTABLE_AV(*mark);
-		const I32 maxarg = AvFILL(av) + 1;
-		(void)POPs; /* get rid of the array ref */
-		EXTEND(SP, maxarg);
-		if (SvRMAGICAL(av)) {
-		    U32 i;
-		    for (i=0; i < (U32)maxarg; i++) {
-			SV ** const svp = av_fetch(av, i, FALSE);
-			SP[i+1] = svp
-			    ? SvGMAGICAL(*svp) ? (mg_get(*svp), *svp) : *svp
-			    : &PL_sv_undef;
-		    }
-		}
-		else {
-		    Copy(AvARRAY(av), SP+1, maxarg, SV*);
-		}
-		SP += maxarg;
-		PUTBACK;
-	    }
 	    if (!CvLVALUE(cx->blk_sub.cv))
 		goto rvalue_array;
 	    EXTEND_MORTAL(SP - newsp);