summaryrefslogtreecommitdiff
path: root/pp_hot.c
Commit message (Collapse)AuthorAgeFilesLines
* Provide as much diagnostic information as possible in "panic: ..." messages.Nicholas Clark2012-01-161-3/+6
| | | | | | | | | | | | | | | The convention is that when the interpreter dies with an internal error, the message starts "panic: ". Historically, many panic messages had been terse fixed strings, which means that the out-of-range values that triggered the panic are lost. Now we try to report these values, as such panics may not be repeatable, and the original error message may be the only diagnostic we get when we try to find the cause. We can't report diagnostics when the panic message is generated by something other than croak(), as we don't have *printf-style format strings. Don't attempt to report values in panics related to *printf buffer overflows, as attempting to format the values to strings may repeat or compound the original error.
* [perl #35865, #43011] FETCH after autovivifyingFather Chrysostomos2012-01-101-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After autovivification, perl should not assume that the value it has assigned to a magical scalar is the one that would have been returned. The result of this assumption is that any tie class that copies things assigned to it will cause autovivification to assign to a temporary aggregate without warning, in cases like this: $tied{nonexistent}{foo} = 7; The hash implicitly assigned to $tied{nonexistent} ends up being freed after the =7 assignment. This commit changes autovivification to do FETCH immediately after doing STORE. This required changing some recently-added tests in gmagic.t. Without this change, you end up with horrific workarounds (using B.pm to get the reference count), like the one in JE::Object (which I’m pasting here, in case it has changed by the time you read this): sub STORE { my($self, $key, $val) = @_; my $global = $self->global; if(ref $val eq 'HASH' && !blessed $val && !%$val && svref_2object($val)->REFCNT == 2) { $val = tie %$val, __PACKAGE__, __PACKAGE__->new( $global); } elsif (ref $val eq 'ARRAY' && !blessed $val && !@$val && svref_2object($val)->REFCNT == 2) { require JE::Object::Array; $val = tie @$val, 'JE::Object::Array', JE::Object::Array->new($global); } $self->prop($key => $global->upgrade($val)) }
* Better fix for perl #107440Father Chrysostomos2012-01-091-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | > > Actually, the simplest solution seem to be to put the av or hv on > > the mortals stack in pp_aassign and pp_undef, rather than in > > [ah]v_undef/clear. > > This makes me nervous. The tmps stack is typically cleared only on > statement boundaries, so we run the risks of > > * user-visible delaying of freeing elements; > * large tmps stack growth might be possible with > certain types of loop that repeatedly assign to an array without > freeing tmps (eg map? I think I fixed most map/grep tmps leakage > a > while back, but there may still be some edge cases). > > Surely an ENTER/SAVEFREESV/LEAVE inside pp_aassign is just as > efficient, > without any attendant risks? > > Also, although pp_aassign and pp_undef are now fixed, the > [ah]v_undef/clear functions aren't, and they're part of the public API > that can be called independently of pp_aassign etc. Ideally they > should > be fixed (so they don't crash in mid-loop), and their documentation > updated to point out that on return, their AV/HV arg may have been > freed. This commit takes care of the first part; it changes pp_aassign to use ENTER/SAVEFREESV/LEAVE and adds the same to h_freeentries (called both by hv_undef and hv_clear), av_undef and av_clear. It effectively reverts the C code part of 9f71cfe6ef2.
* [perl #107440] Save av/hv on mortals stack when clearingFather Chrysostomos2012-01-061-2/+2
| | | | | | | | | | | | | | | | | | | In pp_undef and pp_aassign, we should put the av or hv that is being cleared on the mortals stack (with an increased refcount), so that destructors fired during the clearing do not free the av or hv. I was going to put this in av_undef, etc., but pp_aassign also needs to access the aggregate after clearing it. We still get a crash with that approach. Putting the aggregate on the mortals stack in av_undef, av_clear and h_freeentries would work, too, but might cause the aggregate to leak too far. That may cause problems, e.g., if it is %^H, because it may last until the end of the current compilation unit. Directly inside a runloop (in a pp function), it should be OK to use the mortals stack, as it *will* be cleared ‘soon’. This seems the least intrusive approach.
* [perl #95548] Returned magical temps are not copiedFather Chrysostomos2012-01-041-3/+6
| | | | | | | | | | | return and leavesub, for speed, were not copying temp variables with a refcount of 1, which is fine as long as the fact that it was not cop- ied is not observable. With magical variables, that *can* be observed, so we have to forego the optimisation and copy the variable if it’s magical. This obviously applies only to rvalue subs.
* diag_listed_as galoreFather Chrysostomos2011-12-281-0/+1
| | | | | In two instances, I actually modified to code to avoid %s for a constant string, as it should be faster that way.
* pp_hot.c: diag_listed_as for method errorFather Chrysostomos2011-12-271-0/+1
|
* Fix two (er, four) sub:lvalue { &$x } bugsFather Chrysostomos2011-12-261-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The lvalue context that the last statement of an lvalue subroutine provides, when applied to entersub, causes the ops below the entersub to be complied oddly. Compare regular subs and lvalue subs: $ ./perl -Ilib -MO=Concise,bar,foo -e 'sub bar { &$x } sub foo:lvalue { &$x }' main::bar: 5 <1> leavesub[1 ref] K/REFC,1 ->(end) - <@> lineseq KP ->5 1 <;> nextstate(main 1 -e:1) v ->2 4 <1> entersub[t2] K/TARG ->5 - <1> ex-list K ->4 2 <0> pushmark s ->3 - <1> ex-rv2cv vK ->- - <1> ex-rv2sv sK/1 ->- 3 <#> gvsv[*x] s ->4 main::foo: b <1> leavesublv[1 ref] K/REFC,1 ->(end) - <@> lineseq KP ->b 6 <;> nextstate(main 2 -e:1) v ->7 a <1> entersub[t2] K/LVINTRO,TARG,INARGS ->b - <1> ex-list K ->a 7 <0> pushmark s ->8 9 <1> rv2cv vK/NO() ->a - <1> ex-rv2sv sK/1 ->9 8 <#> gvsv[*x] s ->9 -e syntax OK Notice that, in the second case, the rv2cv is not being optimised away. Under strict mode, this allows a sub call on a string, since rv2cv is not subject to strict refs. It’s this code in op.c:op_lvalue_flags that is to blame: if (kid->op_type != OP_GV) { /* Restore RV2CV to check lvalueness */ restore_2cv: if (kid->op_next && kid->op_next != kid) { /* Happens? */ okid->op_next = kid->op_next; kid->op_next = okid; } else okid->op_next = NULL; okid->op_type = OP_RV2CV; okid->op_targ = 0; okid->op_ppaddr = PL_ppaddr[OP_RV2CV]; okid->op_private |= OPpLVAL_INTRO; okid->op_private &= ~1; break; } This code is a little strange. Using rv2cv to check lvalueness causes the problem with strict refs. The lvalue check could just as well go in entersub. The way this is currently written (and this is something I missed when supposedly fixing lvalue subs), the rv2cv op will reject a non-lvalue subroutine even when the caller is not called in lvalue context. So we actually have two bugs. Presumably the check was done in rv2cv to keep entersub fast. But the code I quoted above is only part of it. There is also a special block to create an rv2cv op anew to deal with method calls. This commit fixes both issues by moving the run-time lvalueness check to entersub. I put it after PUSHSUB for speed in the most common case (when there is no error). PUSHSUB already calls a function (was_lvalue_sub) to determine whether the current sub call is happen- ing in lvalue context. So the check I am adding after it only has to check a couple of flags, instead of calling was_lvalue_sub itself. This also fixes a bug I introduced earlier in the 5.15.x series. This is supposed to die (in fact, I made the mistake earlier of changing tests that were checking for this, but so many tests were wrong back then it was an easy mistake to make): $ ./perl -Ilib -e 'sub bar {$x} sub foo:lvalue { bar}; foo=3' And a fourth bug I discovered when writing tests: sub AUTOLOAD :lvalue { warn autoloading; $x } sub _102486 { warn "called" } &{'_102486'} = 72; warn $x __END__ autoloading at - line 1. 72 at - line 4. And it happens even if there is an lvalue sub defined under that name: sub AUTOLOAD :lvalue { warn autoloading; $x } sub _102486 :lvalue { warn "called" } &{'_102486'} = 72; warn $x __END__ autoloading at - line 1. 72 at - line 4. Since the sub cannot be seen at compile time, the lvalue check in rv2cv, as mentioned above. The autoloading is happening in rv2cv, too, instead of entersub (the code is repeated), but the sub is not checked for definition first. It was put in rv2cv because it had to come before the lvalue check. Putting the latter in entersub lets us delete that repeated autoload code, which is completely wrong anyway.
* Don’t crash when writing to null hash elemFather Chrysostomos2011-12-241-2/+2
| | | | | | | It’s possible for XS code to create hash entries with null values. pp_helem and pp_slice were not taking that into account. In fact, the core produces such hash entries, but they are rarely visible from Perl. It’s good to check for them anyway.
* pp_hot.c: First letter of latin-1 classnames wasn't being checked correctly.Brian Fraser2011-12-151-1/+1
| | | | | | Previously the first letter for latin-1 classnames was being mischecked, only allowing ASCII, which caused an instance of the Unicode Bug for downgradable classnames.
* Call FETCH once for rcatlineFather Chrysostomos2011-11-241-3/+3
|
* amagic_deref_call does not necessitate SPAGAINFather Chrysostomos2011-11-221-1/+0
| | | | | As amagic_deref_call pushes a new stack, PL_stack_sp will always have the same value before and after, so SPAGAIN is unnecessary.
* Remove redundant check in pp_hot.c:pp_entersubFather Chrysostomos2011-11-181-2/+0
| | | | PVGVs are always globs now.
* Fix spelling in commentKarl Williamson2011-10-291-1/+1
|
* Cast to signed before negating, to avoid compiler warningsBrian Fraser2011-10-061-1/+1
|
* pp_hot.c: Make warnings utf8-cleanBrian Fraser2011-10-061-5/+5
|
* pp_hot.c: method_common is UTF-8 aware.Brian Fraser2011-10-061-7/+7
| | | | | | | | | | Not really useful yet, since named methods aren't correctly flagged; that is to access a \x{30cb} method, you'd need to do something like Obj->${\"\x{30cb}"}. Committer’s note: I’m also including one piece of the ‘gv.c and pp_ctl.c warnings’ patch so that the newly-added tests in this commit pass.
* pp_hot.c: pp_entersub UTF8 cleanup.Brian Fraser2011-10-061-2/+2
|
* Merge preinc and postincFather Chrysostomos2011-09-161-3/+6
| | | | | They are almost identical. This gives the compiler less code to digest.
* Make ++ and -- work on glob copiesFather Chrysostomos2011-09-161-1/+1
| | | | These ops considered typeglobs read-only, even if they weren’t.
* remove index offsetting ($[)Zefram2011-09-091-2/+0
| | | | | | $[ remains as a variable. It no longer has compile-time magic. At runtime, it always reads as zero, accepts a write of zero, but dies on writing any other value.
* Enter gv_fetchsv_nomgFather Chrysostomos2011-09-081-8/+2
| | | | | | | | | | | There are so many cases that use this incantation to get around gv_fetchsv’s calling of get-magic-- STRLEN len; const char *name = SvPV_nomg_const(sv,len); gv = gv_fetchpvn_flags(name, len, flags | SvUTF8(sv), type); --that it’s about time we had a shorthand.
* Call get-magic once for CV-to-GV assignmentFather Chrysostomos2011-09-031-2/+8
| | | | | | | pp_rv2gv has already called get-magic, so pp_sassign should not do it at all. This is a regression from 5.8.8.
* For s///r, don't call SvPV_force() on the original value. Resolves #97954.Nicholas Clark2011-08-291-0/+8
| | | | | | | | 8ca8a454f60a417f optimised the implementation of s///r by avoiding an unconditional copy of the original value. However, it introduced a behaviour regression where if original value happened to be one of a few particular types, it could be modified by being forced to a string using SvPV_force(). The substitution was (correctly) performed on a copy of this string.
* &CORE::foo() for close, getc and readlineFather Chrysostomos2011-08-251-3/+7
| | | | | | | This commit allows the subs in the CORE package for close, getc and readline to be called through references and via ampersand syntax. The pp functions are modified to take into account the nulls that coreargs pushes on to the stack to indicate that there is no argument.
* &CORE::foo() for @ and $@ prototypes, except unlinkFather Chrysostomos2011-08-251-0/+1
| | | | | | | | | | | | | | This commit allows the CORE subroutines for functions with @ and $@ prototypes to be called through references and via amper- sand syntax. unlink is not included in this commit, as it requires special casing due to its use of implicit $_. Since these functions require a pushmark, and since it has to come between two things that pp_coreargs does, it’s easiest to flag the coreargs op (with the OPpCOREARGS_PUSHMARK flag added in the previous commit) and call pp_pushmark directly from pp_coreargs.
* Make $class->method work when $class is tiedFather Chrysostomos2011-08-241-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This little script: sub TIESCALAR{bless[]} sub FETCH{warn "fetching"; "main"} sub bolgy { warn 'bolgy' } tie my $a, ""; $a->bolgy; Gives these outputs with various versions of perl: $ pbpaste|perl5.6.2 fetching at - line 2. fetching at - line 2. bolgy at - line 3. $ pbpaste|perl5.8.8 fetching at - line 2. fetching at - line 2. fetching at - line 2. Can't call method "bolgy" without a package or object reference at - line 5. $ pbpaste|perl5.8.9 fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. bolgy at - line 3. $ pbpaste|perl5.10.0 fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. Can't call method "bolgy" without a package or object reference at - line 5. $ pbpaste|perl5.10.1 # also 5.12.x fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. bolgy at - line 3. $ pbpaste|perl5.14.0 fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. fetching at - line 2. Can't locate object method "bolgy" via package "main" (perhaps you forgot to load "main"?) at - line 5. It’s worse than ever in 5.14. What’s happening is that S_method_common is hanging on to the pointer returned by SvPV, while continuing to call get-magic again and again. So the pointer becomes invalid. I think it’s only by accident that it worked in some versions. This commit stops S_method_common from calling get-magic so many times, solving both problems. I’m afraid this conflicts with ongoing work to make method lookup UTF8-clean, but I wanted to make a patch that could be backported.
* [perl #97088] Prevent double get-magic in various casesGerard Goossen2011-08-241-12/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch prevents get-magic from executing twice during autovivifi- cation when the op doing the autovivification is not directly nested inside the dereferencing op. This can happen in cases like this: ${ (), $a } = 1; Previously (as of 5.13.something), the outer op was marked with the OPpDEREFed flag, which indicated that get-magic had already been called by the vivifying op (calling get-magic during vivification is inevitable): $ perl5.14.0 -MO=Concise -e '${ $a } = 1' 8 <@> leave[1 ref] vKP/REFC ->(end) 1 <0> enter ->2 2 <;> nextstate(main 2 -e:1) v:{ ->3 7 <2> sassign vKS/2 ->8 3 <$> const[IV 1] s ->4 6 <1> rv2sv sKRM*/DREFed,1 ->7 <-- right here - <@> scope sK ->6 - <0> ex-nextstate v ->4 5 <1> rv2sv sKM/DREFSV,1 ->6 4 <#> gv[*a] s ->5 -e syntax OK But in the ${()...} example above, there is a list op in the way that prevents the flag from being set inside the peephole optimizer. It’s not even possible to set it correctly in all cases, as in this exam- ple, which would need it both set and not set depending on which branch of the ternary operator is executed: ${ $x ? delete $a[0] : $a[0] } = 1 Instead of setting the OPpDEREFed flag, we now make a non-magic copy of the SV in vivify_ref (the first time get-magic is executed).
* Move pp_enter() and pp_leave() with their friends in pp_ctl.cVincent Pit2011-06-261-76/+0
|
* add do_ncmp fn and make pp_ncmp, pp_eq etc use itDavid Mitchell2011-06-251-61/+10
| | | | | | | | | | | | | | | | | | | | | | Extract most of the body of pp_ncmp() (numeric compare) into a separate function, do_ncmp(), then make the following ops use it: pp_ncmp pp_lt pp_le pp_eq pp_ne pp_ge pp_gt This removes a lot of similar or duplicated code, most of which is dedicated to handling the various combinations of IV verses UV verses NV verses NaN. The various ops first check for, and directly process, the simple and common case of both args being SvIOK_notUV(), and pass the processing on to do_ncmp() otherwise. Benchmarking seems to indicate (but with a lot of noise) that the SvIOK_notUV case is slightly faster than before, and the do_ncmp() branch slightly slower.
* remove unreachable code from various compare opsDavid Mitchell2011-06-251-7/+0
| | | | | | | | | | | | | | | | | | | | | | All the compare ops (such as pp_le), have an initial: tryAMAGICbin_MG(le_amg, AMGf_numeric); The effect of the AMGf_numeric flag is that, if the le overloading fails, but either of the args on the stack is a reference, then that arg is replaced with a temporary non-ref arg that is either the result of '0+' overloading, or is a UV with the numerical value of the ref's address. So by the time the main body of the op is called, neither arg can be a ref. Thus a whole bunch of nearly identical blocks can be removed, which *used* to handle comparing refs: if (SvROK(TOPs) && !SvAMAGIC(TOPs) && SvROK(TOPm1s) && !SvAMAGIC(TOPm1s)) { SP--; SETs(boolSV(SvRV(TOPs) <= SvRV(TOPp1s))); RETURN; }
* For s///r, avoid copying the source early only to edit it in place.Nicholas Clark2011-06-231-40/+41
| | | | | | Instead, take advantage of the "can't edit in place" code path of pp_subst which writes to a new scalar, and that pp_substcont always leaves the original intact, writing to a new scalar.
* Move pp_leavesublv from pp_hot.c to pp_ctl.cFather Chrysostomos2011-06-221-177/+0
| | | | | | This is so that it can share code with pp_return. I was going to move pp_return into pp_hot.c, but it uses static functions that other pp_ functions in pp_ctl.c use.
* Remove the CvLVALUE check from pp_leavesubFather Chrysostomos2011-06-221-11/+1
| | | | As of bb3abb0, this can no longer happen.
* In pp_subst, use a mortal scalar for dstr, instead of SAVEFREESV().Nicholas Clark2011-06-171-2/+1
| | | | | | | | | Creation of the mortal can be done with newSVpvn_flags(), saving 1 function call. Also, the mortal stack uses 1 pointer, whereas the save stack will use 2. It doesn't matter that dstr is now marked SVs_TEMP, as both code paths already gut it, stealing its SvPVX(). The SV head will happen to last a bit longer now, to the next FREETMPS, instead of the the scope pop at the end of pp_subst or pp_substcont.
* [perl #81944] Non-lvalue subs do not copy return valuesFather Chrysostomos2011-06-161-3/+3
| | | | | | | | | | | | | | | | | return and leavesub see if they can cheat by not copying anything marked TEMP, since presumably nothing else is using it. That means the return values of delete() and shift() are not copied. Since @_ aliases to the caller’s variables, sometimes what is returned *is* used elsewhere and still marked TEMP. So cases like sub { return delete $_[0] } ->($x) end up returning $x unchanged, instead of copying it. As mentioned in the ticket, the solution is to copy only if the refer- ence count is 1. This also allows me to simplify the lvalue-returning code without spreading this bug further. (pp_leavesublv currently avoids calling sv_2mortal, in order not to set the TEMP flag.)
* Stop lvalue subs from copying read-only scalarsFather Chrysostomos2011-06-161-3/+1
| | | | | | | | They were only doing so in reference context (subroutine args, for(...)). Explicit return already worked, but only because I didn’t write it well. I’m in the process of trying to merge the two.
* In pp_match, refactor the call to CALLREGEXEC() to avoid a goto.Nicholas Clark2011-06-161-12/+8
| | | | | | The previous slightly contorted logic had an if() block that ended in a goto, where the target was only 6 lines later, and could not be reached directly. It dates back to (at least) 5.000, with no structural changes since then.
* Split OP_AELEMFAST_LEX out from OP_AELEMFAST.Nicholas Clark2011-06-121-1/+1
| | | | | | | | | | | | | 6a077020aea1c5f0 extended the OP_AELEMFAST optimisation to lexical arrays. Previously OP_AELEMFAST was only used as an optimisation for OP_GV, which is a PADOP/SVOP. However, by reusing the same opcode, and signalling (pad) lexical vs package, it introduced a myriad of special cases, because OP_PADAV is a BASEOP (not a PADOP), whilst OP_AELEMFAST is a PADOP/SVOP (which is larger). Using two OP numbers allows each variant to have the correct OP flags in PL_opargs. Both can continue to share the same C code.
* [perl #92290, #92406] Returning a pad var from lv subFather Chrysostomos2011-06-071-2/+10
| | | | | | | | | | | | | | | | | | This fixes a recent (post-5.14.0) regression. Commit bf8fb5e (the fix for #62498) introduced it for lvalue subs with no return statement [perl #92406]. Commit fa1e92c (the fix for #72724) introduced it for lvalue subs that do have an explicit return [perl #92290]. Simply returning a scalar itself from an lvalue sub does not work if it is a pad variable with a reference count of 1. In that circum- stance, the sub-popping code sees that the SV can be re-used the next time the sub is called, so it undefines it and hangs on to it. So the scalar returned gets emptied before the calling code can see it. The reference count has to be increased temporarily, which sv_2mortal and SvREFCNT_inc combined accomplish.
* Allow lvalue subs to return COWs in reference contextFather Chrysostomos2011-06-041-1/+3
| | | | | | | | | | | (That’s ‘reference’ as in ‘pass by reference’. It applies to foo(lvalue_func()) and for(lvalue_func()).) Commit f71f472 took care of scalar context. Commit a0aa607 came and long and took care of list context, but, unfortunately, missed reference context. This commit takes care of that.
* Fix several array-returning bugs in lvalue subsFather Chrysostomos2011-06-041-24/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* [perl #7946] Lvalue subs do not autovivifyFather Chrysostomos2011-06-031-1/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit makes autovivification work with lvalue subs. It follows the same technique used by other autovivifiable ops (aelem, helem, tc.), except that, due to flag constraints, it uses a single flag and instead checks the op tree at run time to find out what sort of thing to vivify. The flag constraints are that these two flags: #define OPpENTERSUB_HASTARG 32 /* Called from OP tree. */ #define OPpENTERSUB_NOMOD 64 /* Immune to op_lvalue() for :attrlist. */ conflict with these: #define OPpDEREF (32|64) /* autovivify: Want ref to something: */ #define OPpDEREF_AV 32 /* Want ref to AV. */ #define OPpDEREF_HV 64 /* Want ref to HV. */ #define OPpDEREF_SV (32|64) /* Want ref to SV. */ Renumbering HASTARG and NOMOD is problematic, as there are places in op.c that change entersubs into rv2cvs, and the entersub and rv2cv flags would conflict. Setting the flags correctly when changing the type is hard and would result in subtle bugs if not done perfectly. Ops like ${...} don’t actually autovivify; it’s the op inside that does it. In those cases, the parent op is flagged with OPpDEREFed, and it skips get-magic, as it has already been called by the inner op. Since entersub is now marked as being an autovivifying op, ${...} in lvalue context ends up skipping get-magic if there is a foo() inside. And this affects even regular subs. So pp_leavesub and pp_return have to call get-magic; hence the new tests in gmagic.t.
* [perl #62498] Scalar context breaks lvalue subsFather Chrysostomos2011-06-011-23/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | That RT ticket reported that a $ prototype puts an implicit scalar() on its argument, and that scalar(lvalue()) causes the function to return a temporary value. In particular: ${\scalar($_)} = 1; # ok ${\scalar f()} = 1; # no effect (where f is an lvalue sub that returns $_). It turns out that this does not only affect scalar(), but also || and &&: ${\($_ && undef)} = 3; # ok ${\(f() && undef)} = 3; # no effect Also, that comment in pp_leavesublv about f()->meth() not being lvalue context is wrong, as $o->${\sub { $_[0] = "whatever" }}; assigns to $o, and sub UNIVERSAL::undef { undef $_[0] } allows calls like $x->undef to undefine $x, if it contains an object or package name. Since copying values in rvalue context is wasteful anyway, since the definition of rvalue context is that the value is going to be copied (resulting in *two* copies), the easiest solution is not to copy val- ues in rvalue context. This ends up applying to what I call ‘reference’ context (semi-lvalue, or potential lvalue) as well. This works already with explicit return. As a bonus, this also fixes bug #78680, for which there are already to-do tests that were added before the bug was reported. See also: http://www.nntp.perl.org/group/perl.perl5.porters/;msgid=20060118203058.GQ616@plum.flirble.org
* Warn when list-assigning to TEMPFather Chrysostomos2011-06-011-0/+8
|
* Make empty lvalue subs work correctlyFather Chrysostomos2011-05-311-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | In perl 5.8.1 and earlier, sub{} would return @_ in list context. This was fixed in 5.8.2 for regular subs, but not lvalue subs. Before the syntactic restriction on return values was removed in commit 145b2bb, there was a bug affecting compilation of empty subs before any use statement: $ perl5.14.0 -e 'sub foo :lvalue {}' Can't modify stub in lvalue subroutine return at -e line 1, near "{}" Execution of -e aborted due to compilation errors. $ perl5.14.0 -le 'use sigtrap; sub foo :lvalue {} print "ok"' ok But I digress. :-) Up to 5.14, lvalue subs were still returning @_, or, rather, the ele- ments of @_ as separate scalars: $ perl5.14.0 -Mre -le '(sub :lvalue {}->($a,$b))=(3,4); print "$a $b"' Useless use of "re" pragma at -e line 0 3 4 (Not exactly useless, eh? The -Mre allows the sub to compile.) This commit fixes that bug.
* Allow lvalue subs to return TEMPsFather Chrysostomos2011-05-311-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is perhaps not ideal, but it fixes (or allows to be fixed) seve- ral bugs. I was hoping that the cases that this perhaps erroneously allows through would fall back to the warning I added in commit 8fe85e3, but, unfortunately, in all these cases the refcount is greater than 1 when pp_sassign is reached. To be less vague: ‘foo() = 3’ warns if foo() returns a TEMP with no set-magic and a refcount of 1 (meaning it will be freed shortly). But truly temporary values returned by pure-Perl lvalue subs have a refer- ence count of at least 2, and as many entries on the mortals stack. I cannot distinguish between truly temporary values and those that are but nominally temporary (marked TEMP because the refcount will go down, but not to zero) by checking for a refcount <= 2 in pp_sassign, because this example returns with a refcount of 2: +sub :lvalue { return delete($_[0]), $x }->($x) = 3; # returns a TEMP There’s no logical reason why that shouldn’t work, if this does: +sub :lvalue { return foo(), $x }->($x) = 3; # not TEMP as they are conceptually identical. The advantages to this change: • The delete example above will work. • It allows XS lvalue subs that return TEMPs to work in the debugger [perl #71172], restoring the bug fix that b724cc1 implemented but c73030b reverted. • It makes these three cases identical, as they should be. Note that only two of them return TEMPs: +sub :lvalue { return shift }->($x) = 3; +sub :lvalue { \@_; return shift }->($x) = 3; # returns a TEMP +sub :lvalue { return delete $_[0] }->($x) = 3; # returns a TEMP So I think the advantages outweigh the disadvantages.
* Revert "Allow returning of temps and ro’s from lv subs"Father Chrysostomos2011-05-311-2/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit b724cc14b25929aee44eee20bd26102cceb520b6. Lvalue subroutines are more of a mess than I realised when I made that commit. I just tried removing the syntactical restriction on what the last statement or the argument to return can be in an lvalue sub. It opened a whole can of worms. PADTMPs are bizarre creatures that have somewhat erratic behaviour. Since assigning to a PADTMP is almost always a mistake (since the value will definitely be discarded), those *should* be disallowed, when the sub is called in lvalue context. That also avoids propagating a whole load of bugs when referencing PADTMPs, aliasing to them, etc. Read-only scalars end up triggering a ‘Modification of a read-only value attempted’ message without the restrictions in pp_leavesublv, but the message the latter was providing (which this revert restores) is clearer (‘Can't return a readonly value from lvalue subroutine’). Speaking of lvalue context, there are three types of context with regard to lvalue-ness (orthogonal to the usual void/scalar/list contexts): • rvalue context ($x = func()) • lvalue context (func() = $x) • semi-lvalue context (\func()) Each is handle by a separate code path in pp_leavesublv: • rvalue context - any value can be returned; it’s copied (waste- ful, perhaps?) • semi-lvalue context - any value can be returned; it’s not copied • lvalue context - stringent rules about what can and cannot be returned (which this revert restores) So it is perfectly fine to restrict what can be returned from an lvalue sub *in true lvalue context*, without affected rvalue use. Now, regarding TEMPs, although this commit restores the restriction on returning TEMPs, a forthcoming commit will relax that restriction once more, since it causes bugs.
* Allow lvalue subs to return COWs in list contextFather Chrysostomos2011-05-301-1/+5
| | | | Commit f71f472 missed list assignment. :-(
* Allow returning of temps and ro’s from lv subsFather Chrysostomos2011-05-301-19/+2
| | | | | | | | | | | | This commit removes the restriction on returning temps and read-only scalars from lvalue subs that occurs when the sub returns implicitly (with no ‘return’ statement; ‘return’ has never had that restriction). It does not actually help pure-Perl lvalue subs much yet, as op.c still enforces lvalue syntax on the last statement. But this should fix bug #71172, allowing XS lvalue subs to work under the debugger.