summaryrefslogtreecommitdiff
path: root/pp_sort.c
Commit message (Collapse)AuthorAgeFilesLines
* OP_SORT: store start of block in null->op_nextDavid Mitchell2014-03-161-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When a sort with a code block, like sort { BLOCK } arg, ... is compiled, it comes out like sort pushmark null scope BLOCK arg ... (The 'scope' may be instead be 'ex-leave' depending on circumstances). At run time, pp_sort() navigates its way down from the sort op to find the start op of the BLOCK. We can shorten this process slightly by storing the start of BLOCK in the otherwise unused op_next field of the OP_NULL. Effectively we are using the null->op_next field as a surrogate op_other field for the op_sort (which doesn't have a spare field we could store the pointer in). The main point of this commit however is not the slight speed up from skipping a couple of pointer follows at run-time; rather that it will shortly allow us to trim any null ops from the beginning of the BLOCK. We can't do this directly, as that would involve changing the scope->op_first pointer, which might confuse B:: type modules.
* don't set SvPADTMP() on PADGV'sDavid Mitchell2014-02-271-1/+3
| | | | | | | | | | | | | | | | | | | | | | | Under threaded builds, GVs for OPs are stored in the pad rather than being directly attached to the op. For some reason, all such GV's were getting the SvPADTMP flag set. There seems to be be no good reason for this, and after skipping setting the flag, all tests still pass. The advantage of not setting this flag is that there are quite a few hot places in the code that do if (SvPADTMP(sv) && !IS_PADGV(sv)) { ... I've replaced them all with if (SvPADTMP(sv)) { assert(!IS_PADGV(sv)); ... Since the IS_PADGV() macro expands to something quite heavyweight, this is quite a saving: for example this commit reduces the size of pp_entersub by 111 bytes.
* perlapi: Consistent spaces after dotsFather Chrysostomos2013-12-291-2/+2
| | | | plus some typo fixes. I probably changed some things in perlintern, too.
* Revert "make perl core quiet under -Wfloat-equal"David Mitchell2013-11-161-1/+1
| | | | | | | | | | | A suggested way of avoiding the the warning on nv1 != nv2 by replacing it with (nv1 < nv2 || nv1 > nv2), has too many issues with NaN. [perl #120538]. I haven't found any other way of selectively disabling the warning, so for now I'm just reverting the whole commit. This reverts commit c279c4550ce59702722d0921739b1a1b92701b0d.
* make perl core quiet under -Wfloat-equalDavid Mitchell2013-11-091-1/+1
| | | | | | | | | | | | | | | | | | | | The gcc option -Wfloat-equal warns when two floating-point numbers are directly compared for equality or inequality, the idea being that this is usually a logic error, and that you should be checking that the values are instead very near to each other. perl on the other hand has lots of reasons to do a direct comparison. Add two macros, NV_eq_nowarn(a,b) and NV_eq_nowarn(a,b) that do the same as (a == b) and (a != b), but without the warnings. They achieve this by instead doing (a < b) || ( a > b). Under gcc at least, this is optimised into the same code as the direct comparison. The are three places that I've left untouched, because they are handling NaNs, and that gets a bit tricky. In particular (nv != nv) is a test for a NaN, and replacing it with (< || >) creates signalling NaNs (whereas == and != create quiet NaNs)
* pp_sort.c: Remove useless assignments; reduce var scopeFather Chrysostomos2013-11-041-3/+1
| | | | | | 6cda7db16df9 stopped the value of the stash variable from being used, so there is no longer any need to assign to it. sv_2cv, however, requires an HV ** argument, so we cannot eliminate it completely.
* Make PL_firstgv and PL_secondgv refcountedFather Chrysostomos2013-10-281-4/+8
| | | | | | | Otherwise freeing *a or *b in a sort block will result in a crash: $ perl -e '@_=sort { delete $::{a}; 3 } 1..3' Segmentation fault: 11
* Correct the citation for Peter McIlroy's sorting paper.John P. Linderman2013-09-121-2/+5
| | | | Also update John P. Linderman's e-mail address.
* Use SSize_t for arraysFather Chrysostomos2013-08-251-1/+1
| | | | | | | | | | Make the array interface 64-bit safe by using SSize_t instead of I32 for array indices. This is based on a patch by Chip Salzenberg. This completes what the previous commit began when it changed av_extend.
* [perl #78194] Make sort copy PADTMPsFather Chrysostomos2013-07-251-0/+4
| | | | | | | | | | Copy PADTMPs (op return values) when there is a sort block/sub that is not optimised away and we are not sorting in place, so that \$a == \$a will return true. Many ops return the same scalar each time, for efficiency; refgen (\) knows about that and copies them, to hide the implementation detail, but other ops (sort in this case) need to do the same thing.
* In-place sort should not leave array read-onlyFather Chrysostomos2013-06-261-0/+3
| | | | | | | | | $ ./perl -Ilib -e '@a=1..2; eval { @a=sort{die} @a }; warn "ok so far\n"; @a = 1' ok so far Modification of a read-only value attempted at -e line 1. If something goes wrong inside the sort block and it dies, we still need to make sure we turn off the read-only flag on that array.
* Stop using PL_sortstashFather Chrysostomos2013-06-081-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Originally, the value of PL_sortstash was used to determine whether PL_firstgv and PL_secondgv (which point to *a and *b in the current package) needed to be updated (involving symbol lookup). PL_sortstash would contain the last package sorted in. If PL_sortstash did not point to the current stash, then they would be updated, along with PL_sortstash. Sort was made reëntrant in 2000 in commit 8e664e1028b. This involved saving and restoring the values of PL_firstgv and PL_secondgv via the savestack. The logic introduced by that commit was thus: + if (PL_sortstash != stash || !PL_firstgv || !PL_secondgv) { + SAVESPTR(PL_firstgv); + SAVESPTR(PL_secondgv); + PL_firstgv = gv_fetchpv("a", TRUE, SVt_PV); + PL_secondgv = gv_fetchpv("b", TRUE, SVt_PV); + PL_sortstash = stash; + } PL_firstgv and PL_secondgv would be null if no sort were already hap- pening, causing this block to be executed. For a nested sort, this would only be executed if the nested sort were in a different package from the outer sort. Because the value of PL_sortstash was not being restored, this block would not trigger the second time the outer sort called the inner sort (in a different package). This was bug #36430 (a duplicate of #7063). This was fixed in commit 9850bf21fc4e, which did not explain anything in its commit message. A preliminary version of the patch was submit- ted for review in <20051026173901.GA3105@rpc142.cs.man.ac.uk>, which mentions that the patch fixes bug #36430. It fixed the bug by localising the value of PL_sortstash, causing the if() condition to become redundant, so that was removed. Now, it happened that that if() condition was the only place that PL_sortstash was used. So if we are going to localise PL_firstgv and PL_secondgv unconditionally when sorting, PL_sortstash serves no purpose. While I could restore the if() condition and leave the localisation of PL_sortstash in place, that would only benefit code that does nested sort calls in the same package, which is rare, and the resulting speed-up is barely measurable. PL_sortstash is referenced by some CPAN modules, so I am taking the cautious route of leaving it in for now, though the core doesn’t use it.
* Change pods to not refer to av_len()Karl Williamson2013-02-081-1/+1
| | | | This name for the function is misleading; don't encourage its use.
* Remove "register" declarationsKarl Williamson2012-11-241-4/+4
| | | | | | | This finishes the removal of register declarations started by eb578fdb5569b91c28466a4d1939e381ff6ceaf4. It neglected the ones in function parameter declarations, and didn't include things in dist, ext, and lib, which this does include
* Fix panic/crash with sort { $not_num } and fatal warningsFather Chrysostomos2012-11-201-5/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I caused this in 5.15.4 in commit 1aa032b25ab: $ ./miniperl -Ilib -e 'eval q|use warnings FATAL=>all=>; ()=sort{undef}1,2|' panic: illegal pad in SAVEt_FREEOP: 0x803500[0x0] at -e line 1. This panic only happens under debugging builds. But it’s worse than that: $ ./miniperl -Ilib -e 'eval { use warnings FATAL => all=>; ()=sort{undef}1,2}; my $x' Bus error It’s this piece of code in pp_sort.c that is the problem: pad = PL_curpad; PL_curpad = 0; if (PL_stack_sp != PL_stack_base + 1) { assert(PL_stack_sp == PL_stack_base); result = SvIV(&PL_sv_undef); } else result = SvIV(*PL_stack_sp); PL_curpad = pad; If SvIV dies, then PL_curpad will never be restored. That results in a panic error when the string eval exits, under debugging builds, and a crash for any subsequent pad ops, under any build. So we need to use the savestack to protect PL_curpad. To avoid the overhead most of the time, we should do this only if the result is not already a number. Sorting with a sub that has a ($$) prototype follows a different code path that contains the same logic, but it is safe in that case, because sort with a sub already localises the pad. I added tests for it anyway.
* rmv context from Perl_croak_no_modify and Perl_croak_xs_usageDaniel Dragan2012-11-121-1/+1
| | | | | | | | | | | Remove the context/pTHX from Perl_croak_no_modify and Perl_croak_xs_usage. For croak_no_modify, it now has no parameters (and always has been no return), and on some compilers will now be optimized to a conditional jump. For Perl_croak_xs_usage one push asm opcode is removed at the caller. For both funcs, their footprint in their callers (which probably are hot code) is smaller, which means a tiny bit more room in the cache. My text section went from 0xC1A2F to 0xC198F after apply this. Also see http://www.nntp.perl.org/group/perl.perl5.porters/2012/11/msg195233.html .
* Use PADLIST in more placesFather Chrysostomos2012-08-211-1/+1
| | | | | Much code relies on the fact that PADLIST is typedeffed as AV. PADLIST should be treated as a distinct type.
* Omnibus removal of register declarationsKarl Williamson2012-08-181-13/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This removes most register declarations in C code (and accompanying documentation) in the Perl core. Retained are those in the ext directory, Configure, and those that are associated with assembly language. See: http://stackoverflow.com/questions/314994/whats-a-good-example-of-register-variable-usage-in-c which says, in part: There is no good example of register usage when using modern compilers (read: last 10+ years) because it almost never does any good and can do some bad. When you use register, you are telling the compiler "I know how to optimize my code better than you do" which is almost never the case. One of three things can happen when you use register: The compiler ignores it, this is most likely. In this case the only harm is that you cannot take the address of the variable in the code. The compiler honors your request and as a result the code runs slower. The compiler honors your request and the code runs faster, this is the least likely scenario. Even if one compiler produces better code when you use register, there is no reason to believe another will do the same. If you have some critical code that the compiler is not optimizing well enough your best bet is probably to use assembler for that part anyway but of course do the appropriate profiling to verify the generated code is really a problem first.
* update the editor hints for spaces, not tabsRicardo Signes2012-05-291-2/+2
| | | | | This updates the editor hints in our files for Emacs and vim to request that tabs be inserted as spaces.
* Make sort {} and sort {()} equivalentFather Chrysostomos2011-11-211-6/+10
| | | | | | | | | | sub {} and sub{()} are equivalent. In list context they both return the empty list. In scalar context they both return undef. But sort doesn’t seem to think so. It croaks on sub{}. This commit fixes that and makes it consistent. I left XSUBs alone, since I’m not sure how they are supposed to behave.
* Make sort’s warnings dependent on the right hintsFather Chrysostomos2011-11-191-0/+4
| | | | | | | | | | | | | | sort’s warnings about uninitialized (or non-numeric) values returned from comparison routines are emitted in the scope of the compar- ison routine, not the sort function itself. So, not only does ‘use warnings; sort...’ not always warn, but the line numbers can be off, too: $ ./perl -Ilib -e '()=sort flobbp 1,2;' -e'use warnings;sub flobbp{"foo"}' Argument "foo" isn't numeric in sort at -e line 2. The solution is to restore PL_curcop to its previous value before get- ting a number out of the comparison routine’s return value.
* Fix VC6 compilation of pp_sort.cSteve Hay2011-11-071-1/+1
| | | | Fixes copy & paste errors introduced by f3dab52a51.
* Make XS sort routines work againFather Chrysostomos2011-10-151-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These stopped working when the CvROOT and CvXSUB fields were merged in 5.10.0: $ perl5.8.9 -le 'print sort utf8::is_utf8 2,1' Usage: utf8::is_utf8(sv) at -e line 1. $ perl5.10.0 -le 'print sort utf8::is_utf8 2,1' 12 (In the latter case, the utf8::is_utf8 routine is not being called.) pp_sort has this: if (!(cv && CvROOT(cv))) { if (cv && CvISXSUB(cv)) { But CvROOT is the same as CvXSUB, so that block is never entered for XSUBs, so this piece of code later on: if (is_xsub) PL_sortcop = (OP*)cv; else PL_sortcop = CvSTART(cv); sets PL_sortcop to CvSTART for XSUBs, but CvSTART is NULL. Later on, this if condition fails: if (PL_sortcop) { so the XSUB is treated as being absent.
* Stop uninit sort warnings from crashingFather Chrysostomos2011-10-131-0/+6
| | | | | | | | | | | | | | | | | | | | | | Commit d4c6760a made the warning in cases like this mention the sort operator: $ ./miniperl -we '()=sort { undef } 1,2' Use of uninitialized value [in sort] at -e line 1. It did so by setting PL_op during the SvIV(retval of sort block). But sv.c:S_find_uninit_var, called by report_uninit, tries to access the targets of some ops, which are in PL_curpad on threaded builds. In the case of a sort sub (rather than an inlined block), PL_curpad con- tained whatever was left over from the sort block (I presume, but have not confirmed; in any case what is in PL_curpad is bad), causing find_uninit_var to crash. This commit sets PL_curpad to null and puts a check for it in report_uninit. It did not crash in debugging threaded builds, but that was probably luck (even though I don’t believe in it).
* [perl #94390] Optimised numeric sort should warn for nanFather Chrysostomos2011-10-121-0/+8
| | | | | | | | | | | | | | | In this case: sort { $a <=> $b } ... the sort block is optimised away and implemented in C. That C implementation did not take into account that $a or $b might be nan, and therefore, without optimisation, would return undef, result- ing in a warning. The optimisation is supposed to be just that, and not change behaviour.
* Mention sort in warnings about sort sub retvalsFather Chrysostomos2011-10-121-0/+4
| | | | | | | | | | | | With this commit, $ ./miniperl -we '()=sort { undef } 1,2' Use of uninitialized value at -e line 1. becomes $ ./miniperl -we '()=sort { undef } 1,2' Use of uninitialized value in sort at -e line 1.
* [perl #30661] autoload sort subsFather Chrysostomos2011-10-111-1/+19
|
* Fix typos (spelling errors) in Perl sources.Peter J. Acklam) (via RT2011-01-071-5/+5
| | | | | | | | | # New Ticket Created by (Peter J. Acklam) # Please include the string: [perl #81904] # in the subject line of all future correspondence about this issue. # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=81904 > Signed-off-by: Abigail <abigail@abigail.be>
* standardise amagic method namingDavid Mitchell2010-12-311-5/+5
| | | | | | | | | | | | | | Some amagic-related macros take the full method enumeration name, (e.g. "add_amg"); while others "helpfully" allow you to pass a shortened version, ("add"), and do a CAT2(meth,_amg) behind the scenes. Standardise on passing the full name; this makes it less confusing and allows you to grep for the enumeration name in the source. It updates two macros to accept full enumeration names: tryAMAGICunTARGET (which isn't used outside the core apparently), and AMG_CALLun, which is replaced by a new AMG_CALLunary (since AMG_CALLun is used outside the core).
* [perl #76026] match variables persist between calls to a sort subFather Chrysostomos2010-12-111-0/+6
| | | | | | | | | | | | Since, for speed’s sake, pp_sort does not call PUSH/POPBLOCK for every invocation of a sort subroutine, it fails to restore PL_curpm after each call (POPBLOCK usually handles that). So the new values of match vars like $1 when the sub returns are what it sees at the next invocation. This commit fixes this by resetting PL_curpm after each call to the subroutine. There are actually three different functions for this (S_sortcv*) so they all need modification.
* [perl #77930] cx_stack reallocation during sortFather Chrysostomos2010-09-201-0/+3
| | | | | Reset cx in pp_sort before POPSUB, as the pointer may no longer be valid.
* PL_amagic_generation doesn't show overload loadedDavid Mitchell2010-07-031-1/+1
| | | | | | PL_amagic_generation is non-zero even without the presence of 'use overload', so don't bother using it as a short-cut test of whether we can skip AMAGIC processing
* Add Perl_croak_no_modify() to implement Perl_croak("%s", PL_no_modify).Nicholas Clark2010-06-271-1/+1
| | | | | This reduces object code size, reducing CPU cache pressure on the non-exception paths.
* In pp_sort, ensure that @_ is freed correctly.Nicholas Clark2010-06-241-3/+3
| | | | Before this, if @_ had become AvREAL(), it retains reference on its elements.
* In S_sortcv_stacked(), handle @_ correctly. Fix for #72334.Gerard Goossen2010-06-231-1/+7
| | | | Remove AvREAL from @_, and set AvALLOC when reallocating @_.
* RT #34604 didn't honour tied overloaded valuesDavid Mitchell2010-05-081-19/+9
| | | | | | A tied hash lookup could return an overloaded object but sort wouldn't notice that it was overloaded because it checked for overload before doing mg_get().
* [perl #71076] sort with active sub (5.10 regression)Father Chrysostomos2009-12-071-0/+5
| | | | | | | | | One of the tests in sort.t causes a bus error (or sometimes ‘Undefined subroutine called’) if run multiple times. This is because sort decreases the refcount of an active sub used as a comparison routine. Ironically enough, this test was added by the very change that broke it (25953/9850bf2).
* pp_sort.c typo: stabiltyReini Urban2009-12-031-1/+1
| | | | | | | | | | | | | | I'm now working on the sort code in th perl compiler as you can see :) -- Reini Urban http://phpwiki.org/ http://murbreak.at/ From b8c749be70f51499fe1ffd9e483ee3a0a8305d9b Mon Sep 17 00:00:00 2001 From: Reini Urban <rurban@cpan.org> Date: Thu, 3 Dec 2009 19:44:57 +0000 Subject: [PATCH] fix typo: stabilty Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>
* [perl #69384] numericness failure in sortingZefram2009-09-271-6/+0
| | | | | This patch removes the error "Sort subroutine didn't return a numeric value" and adds a regression test.
* PATCH: Large omnibus patch to clean up the JRRT quotesTom Christiansen2008-11-021-2/+4
| | | | | | Message-ID: <25940.1225611819@chthon> Date: Sun, 02 Nov 2008 01:43:39 -0600 p4raw-id: //depot/perl@34698
* explicit empty while loopsRobin Barker2008-11-021-1/+1
| | | | | | From: "Robin Barker" <Robin.Barker@npl.co.uk> Message-ID: <46A0F33545E63740BC7563DE59CA9C6D4E2FD9@exchsvr2.npl.ad.local> p4raw-id: //depot/perl@34695
* Explicitly specify some printf formats for constant strings.Rafael Garcia-Suarez2008-11-021-1/+1
| | | | | | This is mostly to silence gcc's warning, "format not a string literal and no format arguments". p4raw-id: //depot/perl@34694
* Eliminate (SV *) casts from the rest of *.c, picking up one (further)Nicholas Clark2008-10-301-1/+1
| | | | | erroneous const in dump.c. p4raw-id: //depot/perl@34675
* Eliminate (AV *) casts in *.c.Nicholas Clark2008-10-291-3/+3
| | | p4raw-id: //depot/perl@34650
* Add MUTABLE_CV(), and eliminate (CV *) casts in *.c.Nicholas Clark2008-10-291-1/+1
| | | p4raw-id: //depot/perl@34647
* Update copyright years.Nicholas Clark2008-10-251-2/+2
| | | p4raw-id: //depot/perl@34585
* [perl #54758] Perl 5.10 memory corruptionDave Mitchell2008-05-271-2/+3
| | | | | | | When @a = sort @a is pessimised if @a has magic, growing the stack requires various pointers to be reset in case the stack gets reallocated. p4raw-id: //depot/perl@33937
* Silence some warnings on Win32 with VC6Steve Hay2008-03-061-12/+12
| | | | | | | | VC7 onwards didn't seem to mind (perhaps thanks to #33411): http://www.nntp.perl.org/group/perl.daily-build.reports/2008/03/msg54118.html but VC6 wasn't happy: http://www.nntp.perl.org/group/perl.daily-build.reports/2008/03/msg54099.html p4raw-id: //depot/perl@33448
* assert() that every NN argument is not NULL. Otherwise we have theNicholas Clark2008-02-121-2/+31
| | | | | | | | | | | | ability to create landmines that will explode under someone in the future when they upgrade their compiler to one with better optimisation. We've already done this at least twice. (Yes, some of the assertions are after code that would already have SEGVd because it already deferences a pointer, but they are put in to make it easier to automate checking that each and every case is covered.) Add a tool, checkARGS_ASSERT.pl, to check that every case is covered. p4raw-id: //depot/perl@33291
* Update copyright years in .c filesRafael Garcia-Suarez2007-01-051-1/+1
| | | p4raw-id: //depot/perl@29696