This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
fix issues in hash assignment with odd elements
authorRuslan Zakirov <ruz@bestpractical.com>
Mon, 8 Oct 2012 13:32:24 +0000 (17:32 +0400)
committerFather Chrysostomos <sprout@cpan.org>
Tue, 11 Dec 2012 16:59:40 +0000 (08:59 -0800)
1) unify code path of processing odd case with regular
loop that process pairs. fixes memory leak when key
is magic and dies on fetch.

2) in list context put undef that we added to compensate
odd elements into returned values, so (%h = (1))
returns (1, $h{1}) rather than (1). This is documented
in perldoc perlop:

    a list assignment in list context produces the list
    of lvalues assigned to.

Here can be a dispute, but:

* that key without value on RHS is still assigned with
  undef
* consider (%h = (1,2,3,4,5,6,3)). Returning (1,2,3,undef,5,6)
  is much easier than (1,2,5,6,3).
* order of returned elements follows cases with even number
  of elements on RHS and duplicates

3) hash assignment with duplicates and odd elements on
RHS was returning wrong results in list context.
Now (%h = (1,1,1)) returns (1,$h{1}).

pp_hot.c

index df93228..54f1eec 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -957,9 +957,6 @@ S_do_oddball(pTHX_ HV *hash, SV **relem, SV **firstrelem)
     PERL_ARGS_ASSERT_DO_ODDBALL;
 
     if (*relem) {
-       SV *tmpstr;
-        const HE *didstore;
-
         if (ckWARN(WARN_MISC)) {
            const char *err;
            if (relem == firstrelem &&
@@ -974,15 +971,6 @@ S_do_oddball(pTHX_ HV *hash, SV **relem, SV **firstrelem)
            Perl_warner(aTHX_ packWARN(WARN_MISC), "%s", err);
        }
 
-        tmpstr = newSV(0);
-        didstore = hv_store_ent(hash,*relem,tmpstr,0);
-        if (SvMAGICAL(hash)) {
-            if (SvSMAGICAL(tmpstr))
-                mg_set(tmpstr);
-            if (!didstore)
-                sv_2mortal(tmpstr);
-        }
-        TAINT_NOT;
     }
 }
 
@@ -1097,6 +1085,7 @@ PP(pp_aassign)
 
                while (relem < lastrelem) {     /* gobble up all the rest */
                    HE *didstore;
+                   ODD:
                    sv = *relem ? *relem : &PL_sv_no;
                    relem++;
                    tmpstr = sv_newmortal();
@@ -1125,7 +1114,9 @@ PP(pp_aassign)
                }
                if (relem == lastrelem) {
                    do_oddball(hash, relem, firsthashrelem);
-                   relem++;
+                    /* we have lelem to reuse, it's not needed anymore */
+                    *(relem+1) = NULL;
+                   goto ODD;
                }
                LEAVE;
            }
@@ -1249,14 +1240,14 @@ PP(pp_aassign)
                 * obliterates the earlier key. So refresh all values. */
                lastrelem -= duplicates;
                relem = firsthashrelem;
-               while (relem < lastrelem) {
+               while (relem <= lastrelem) {
                    HE *he;
                    sv = *relem++;
                    he = hv_fetch_ent(hash, sv, 0, 0);
                    *relem++ = (he ? HeVAL(he) : &PL_sv_undef);
                }
            }
-           SP = lastrelem;
+           SP = ((lastrelem - firsthashrelem)&1)? lastrelem : lastrelem+1;
        }
        else
            SP = firstrelem + (lastlelem - firstlelem);