This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
perl5.git
6 years agosv.c: Clarify some comments
Karl Williamson [Thu, 8 Jun 2017 03:41:12 +0000 (21:41 -0600)]
sv.c: Clarify some comments

6 years agoAdd XS-callable function is_utf8_invariant_string_loc()
Karl Williamson [Tue, 6 Jun 2017 00:33:05 +0000 (18:33 -0600)]
Add XS-callable function is_utf8_invariant_string_loc()

This is like is_utf8_invariant_string(), but takes an additional
parameter, a pointer into which it stores the location of the first
variant if any are found.

6 years agobytes_to_utf8(): Remove obsolete comment
Karl Williamson [Mon, 5 Jun 2017 18:56:28 +0000 (12:56 -0600)]
bytes_to_utf8(): Remove obsolete comment

It said the logic was duplicated elsewhere, but now the essence of the
logic is in an inlined function used in both places.

6 years agoutf8.c: White_space only
Karl Williamson [Thu, 8 Jun 2017 03:17:56 +0000 (21:17 -0600)]
utf8.c: White_space only

The previous commit created a block around the lines changed by this
current commit.

6 years agobytes_from_utf8(): Use memcpy if all invariant
Karl Williamson [Mon, 5 Jun 2017 18:26:06 +0000 (12:26 -0600)]
bytes_from_utf8(): Use memcpy if all invariant

This function does two passes over the input.  In the first it decides
if the string can be downgraded, and computes the size needed for the
downgraded string.  In the 2nd pass, it does the conversion.

Adding a single 'if' to the function can bypass the 2nd pass completely
if only invariants are found.  The 2nd pass is replaced by a memcpy().

6 years agoutf8.c: A byte count should be Size_t, not I32
Karl Williamson [Mon, 5 Jun 2017 18:24:39 +0000 (12:24 -0600)]
utf8.c: A byte count should be Size_t, not I32

(or STRLEN.  I thought we were converting to Size_t, so that's what I
chose here.)

6 years agoUpdate test skips for no fchdir.
Craig A. Berry [Thu, 8 Jun 2017 13:53:48 +0000 (08:53 -0500)]
Update test skips for no fchdir.

Follow-up to 489c16bfa14d46.  There are fewer tests now, so skip
fewer when skipping is necessary.

6 years ago[perl #131221] sv_dup/sv_dup_inc are only available under threads
Tony Cook [Thu, 8 Jun 2017 01:06:39 +0000 (11:06 +1000)]
[perl #131221] sv_dup/sv_dup_inc are only available under threads

6 years agoAdd Jacques Germishuys to AUTHORS
Steve Hay [Wed, 7 Jun 2017 21:55:28 +0000 (22:55 +0100)]
Add Jacques Germishuys to AUTHORS

6 years agocorrect PREMSVC80 typo
Jacques Germishuys [Sat, 3 Jun 2017 09:16:06 +0000 (11:16 +0200)]
correct PREMSVC80 typo

6 years agoMove forward in time.
Abigail [Wed, 7 Jun 2017 21:56:09 +0000 (23:56 +0200)]
Move forward in time.

Now that all the deprecations which were marked to be fatal in 5.28
are actually fatal, it's time to update the wording of the Perl 5.28
section of pod/perldeprecation. Things will not disappear, they are
gone now.

6 years agodoop.c: White-space only
Karl Williamson [Wed, 7 Jun 2017 21:40:33 +0000 (15:40 -0600)]
doop.c: White-space only

Outdent, since the previous commit removed an enclosing block

6 years agoUse simple-minded approach to bitwise UTF-8 operations
Karl Williamson [Wed, 7 Jun 2017 20:52:52 +0000 (14:52 -0600)]
Use simple-minded approach to bitwise UTF-8 operations

Commit 5d09ee1cb7b68f5e6fd15233bfe5048612e8f949 fatalized bitwise
operations of operands with wide characters in them.  It retained the
regular UTF-8 handling, but throws an error when a wide character is
encountered.

But this code is complicated because of its original intended
generality.  It can essentially be ripped out, replaced by code that
just downgrades the operand to non-UTF-8.  Then we use the regular code
to do the operation.  In the complement case, that's all that need be
done to mimic earlier behavior, as the result has not been in UTF-8.
For the other operations, the result is simply upgraded to UTF-8.

This removes quite a few lines of code, and now the UTF-8 handling uses
the same tight loops as the non-UTF-8.  Downgrading and upgrading had to
be done specially before, but now they are done in tight loops, before
the operation, and after the operation

6 years agot/op/bop.t: Re-add in some tests
Karl Williamson [Wed, 7 Jun 2017 20:20:08 +0000 (14:20 -0600)]
t/op/bop.t: Re-add in some tests

Commit 5d09ee1cb7b68f5e6fd15233bfe5048612e8f949 fatalized above 0xFF
code points in the bitwise operators.  It removed a bunch of tests in
t/op/bop.t.  I had independently been working on this task.  An issue
with removing such tests is are they there to be testing wide
characters, or are they there to test UTF-8, and the use of above-FF was
merely incidental?  I had undertaken the tedious work of looking through
the commits for all the tests that now fail to determine which category
they were in.  I concur mostly with the previous commit, but there were
a few tests that I found that should have been retained, and modified to
pass under the new scheme.  This commit does that.

Deprecated warnings no longer have to be turned off, as they were
because of the previous tests that have now been removed.

6 years agoUpdate pods about bitwise UTF-8 above 0xFF being fatal
Karl Williamson [Wed, 7 Jun 2017 19:31:10 +0000 (13:31 -0600)]
Update pods about bitwise UTF-8 above 0xFF being fatal

6 years agot/op/bop.t: Verify complement downgrades UTF-8.
Karl Williamson [Sat, 3 Jun 2017 14:34:56 +0000 (08:34 -0600)]
t/op/bop.t: Verify complement downgrades UTF-8.

This adds a test for the existing result, so it doesn't get changed in
the future by mistake.  I make no claims that it should work this way.

6 years agot/op/bop.t: Verify bitwise & ^ | retain UTF-8
Karl Williamson [Wed, 7 Jun 2017 19:57:41 +0000 (13:57 -0600)]
t/op/bop.t: Verify bitwise & ^ | retain UTF-8

If one of the inputs is encoded in UTF-8, the behavior has been to have
the output be also in UTF-8.  This has not been really documented, but I
think that is the way it should be.  In any case, this commit adds tests
to make sure it doesn't inadvertently get changed.

6 years agoFix Windows build following commit 9d5e3f1aaa
Steve Hay [Wed, 7 Jun 2017 16:58:11 +0000 (17:58 +0100)]
Fix Windows build following commit 9d5e3f1aaa

6 years agoMake setting ${^ENCODING} to a defined value fatal
Dagfinn Ilmari Mannsåker [Thu, 1 Jun 2017 17:16:56 +0000 (18:16 +0100)]
Make setting ${^ENCODING} to a defined value fatal

This has been deprecated since 5.22 and a no-op since 5.26.

Remove the now-obsolete t/uni/heavy.t test, which only tested that
utf8_heavy.pl didn't fail to load when ${^ENCODING} was set.

6 years agoperly.y: add $$ = 0 for midrule code blocks
David Mitchell [Wed, 8 Mar 2017 14:57:23 +0000 (14:57 +0000)]
perly.y: add $$ = 0 for midrule code blocks

In places where a rule contains multiple code blocks, ensure that
$$ is assigned a valid value at the end of midrule blocks, so that

    valgrind ./perl -Dpv ...

doesn't display zillions of

    Conditional jump or move depends on uninitialised value

errors, when perl tries to display the parse stack.

I've only done the various newish top-level grammar entries - these all
seemed to have the same defect, while from a quick glance elsewhere in the
file, it seemed like older rules already do this.

6 years ago[MERGE] rework Perl_sv_vcatpvfn_flags
David Mitchell [Wed, 7 Jun 2017 10:09:38 +0000 (11:09 +0100)]
[MERGE] rework Perl_sv_vcatpvfn_flags

Bug fixes:

    Size modifiers with integer Inf/Nan, e.g. sprintf("%hi", Inf)
    used to print spurious warnings.

    The very-special-cased "%.NNNg" code has been removed, and is now
    handled by the  special-cased "...%.NNNg..." code instead, which makes
    it about 4% slower, but handles unicode radix points better.

    "%n" with a missing arg now croaks with a specific error message
    (previously it croaked about modifying a read-only variable).

    sprintf("%p", 0+Inf) now prints the address of an SV, not the literal
    string "Inf" (or "NaN").

    It now warns on a missing explicit width or vector arg, e.g. "%*2$d"
    or "%3$vd".

    Widths and precisions specified via an argument used to wrap; for
    example, this used to output "  abc"; it now croaks with "Integer
    overflow in format string":

        $w = 0x100000005; printf "%*s", $w, "abc";

    Illegal formats didn't work well with utf8. It now resets to the
    character following the '%' and continues from there.

    The locale-specific radix point code now uses
    SvCUR(PL_numeric_radix_sv) rather than SvLEN(PL_numeric_radix_sv) for
    buffer size allocation.  Also, several places weren't taking into
    account the length of the radix point string when calculating buffer
    sizes; they were only being saved by the general 'add 40' fudge
    factor.

Buffer sizing and length calculation overflow/wraps:

    The code has been thoroughly audited to look for any issues where the
    format and/or args are malicious, and some possible (although not
    exploitable) wraps have been fixed.

    int's and I32's etc have been replaced with STRLEN where appropriate.

    Tests for overflow/wrap have been added to the test suite.

    The code has been cleaned up a lot and better commented, so it will be
    easier to avoid wrapping in future.

Optimisations:

    The special-cased "...%.0f..." is now detected and handled earlier in
    the loop, so is about 35% faster.

    It now sets and restores locale only once per function call, not
    multiple times per format element.

    Generally the code has been heavily tweaked.

    On average, the newly-added benchmarks are about 10-15% faster; the
    mixed-utf8 benchmark is about about 80% faster.

Other changes:

    The HAS_LDBL_SPRINTF_BUG code that worked around a 2002 Irix 6
    bug has been removed.

    The 300-line block of code which handles %a hex floating-point formats
    has been moved into a separate static function.

    The 'svmax' arg to the sv_vcatpvfn() family has been changed from I32
    to STRLEN and renamed to 'sv_count'.

    A "Redundant argument in printf" warning is no longer emitted after an
    invalid % format element has been encountered. This makes the code
    simpler, and since you get an "Invalid conversion in printf" warning
    anyway, does no harm (and in fact may be less confusing).

    Several locals vars have been eliminated and the scope of many others
    has been reduced.

6 years agoadd some sprintf benchmarks
David Mitchell [Fri, 2 Jun 2017 14:57:29 +0000 (15:57 +0100)]
add some sprintf benchmarks

6 years agoPerl_sv_vcatpvfn_flags: rename a label
David Mitchell [Fri, 2 Jun 2017 14:12:46 +0000 (15:12 +0100)]
Perl_sv_vcatpvfn_flags: rename a label

s/donevalidconversion/done_valid_conversion/

so its a bit easier to read.

6 years agosv_vcatpvfn_flags and wrappers: s/svmax/sv_count/
David Mitchell [Fri, 2 Jun 2017 14:07:10 +0000 (15:07 +0100)]
sv_vcatpvfn_flags and wrappers: s/svmax/sv_count/

Rename the 'svmax' parameter of

    Perl_sv_vcatpvfn_flags(),
    Perl_sv_vcatpvfn(),
    Perl_sv_vsetpvfn(),

to 'sv_count'.

'max' often implies N-1 (e.g. svarsg[0]..svargs[svmax]), whereas it's
actually the number of SV args passed to the functions.

6 years agoPerl_sv_vcatpvfn_flags: handle mixed utf8 better
David Mitchell [Fri, 2 Jun 2017 13:47:11 +0000 (14:47 +0100)]
Perl_sv_vcatpvfn_flags: handle mixed utf8 better

Once the output string gets upgraded to utf8 (e.g. due to a utf8 %s
argument), any remaining appending of plain (non-%) parts of the
format string becomes very inefficient. It basically creates an
SV out of the next format chunk, upgrades that SV to utf8, then
appends the upgraded buffer.

This commits makes it just append the format chunk byte by byte, upgrading
in the fly if that byte is !NATIVE_BYTE_IS_INVARIANT

6 years agoadd S_sv_catpvn_simple() for use by sprintf
David Mitchell [Fri, 2 Jun 2017 12:51:45 +0000 (13:51 +0100)]
add S_sv_catpvn_simple() for use by sprintf

Currently Perl_sv_vcatpvfn_flags() uses an unrolled sv_catpvn_nomg()
to append floating point formats, a call to sv_catpvn_nomg() to append
non-% parts of the format, and a few other non-performance-critical
calls to sv_catpvn_nomg().

Move the unrolled code block into an inline static function, and make
the non-% appending use it too.

6 years agoPerl_sv_vcatpvfn_flags: re-indent a code block
David Mitchell [Fri, 2 Jun 2017 12:08:12 +0000 (13:08 +0100)]
Perl_sv_vcatpvfn_flags: re-indent a code block

whitespace only

6 years agoPerl_sv_vcatpvfn_flags: eliminate p var
David Mitchell [Fri, 2 Jun 2017 12:00:52 +0000 (13:00 +0100)]
Perl_sv_vcatpvfn_flags: eliminate p var

It has 1500-line scope, and is equal to fmtstart-1 for most of the
time.

This also allows us to 'const'ify some variables better.

6 years agoPerl_sv_vcatpvfn_flags: clarify GCC bug comments
David Mitchell [Fri, 2 Jun 2017 11:23:32 +0000 (12:23 +0100)]
Perl_sv_vcatpvfn_flags: clarify GCC bug comments

In particular it wasn't clear what bug was being worked around, nor that
'#13488' referred to a GNU ticket rather than a perl ticket.

This bug was fixed back in 2004, but the workaround is fairly harmless, so
I've left it as-is.

6 years agoPerl_sv_vcatpvfn_flags: simplify alt handling
David Mitchell [Fri, 2 Jun 2017 10:57:11 +0000 (11:57 +0100)]
Perl_sv_vcatpvfn_flags: simplify alt handling

only do calculations for alt (#) formatting in the branches which use it

6 years agoPerl_sv_vcatpvfn_flags: rename 'p' var 's'
David Mitchell [Fri, 2 Jun 2017 10:41:41 +0000 (11:41 +0100)]
Perl_sv_vcatpvfn_flags: rename 'p' var 's'

In the 'append # block of code at the end of the loop, don't re-use the
widely-scoped 'p' pointer; instead use a tightly scope var instead
(named 's' do it doesn't clash with p which is still valid in an outer
scope.)

6 years agoPerl_sv_vcatpvfn_flags: simplify format appending
David Mitchell [Fri, 2 Jun 2017 08:51:40 +0000 (09:51 +0100)]
Perl_sv_vcatpvfn_flags: simplify format appending

The bit at the end of the main loop has a whole bunch of conditionals
along the lines of

    if (gap && !left)
         apppend gap
    if (esignlen && !fill)
         append esignbuf
    if (zeros)
        append zeroes
    if (elen)
        append ebuf
    if (gap && left)
        append gap

This involves many tests along the main code path to cope with all the
possibilities (e.g. if left, gap is output before ebuf, otherwise after)

Instead split it into a couple of major branches with duplication between
the branches, but requiring few tests along any one code path.

For example, sprintf("%5d", -1) formerly required 9 branches, 1 for loop,
and 1 memset(). It now requires 2 branches and 3 for loops,

I've removed memset()s and replaced them with for loops. For the short
padding typically used (e.g. "%9d" rather than "%8192d") a loop is faster.

6 years agoPerl_sv_vcatpvfn_flags: eliminate a wrap check
David Mitchell [Thu, 1 Jun 2017 15:05:59 +0000 (16:05 +0100)]
Perl_sv_vcatpvfn_flags: eliminate a wrap check

This is one case where it can never wrap, so don't check.

6 years agoPerl_sv_vcatpvfn_flags: simpler special formats
David Mitchell [Thu, 1 Jun 2017 11:46:23 +0000 (12:46 +0100)]
Perl_sv_vcatpvfn_flags: simpler special formats

At the top of Perl_sv_vcatpvfn_flags(), certain fixed formats are
special-cased: "", "%s", "%-p", "%.0f".

Simplify the code which handles these. In particular, don't try to issue
"missing" or "redundant" arg warnings there. Instead, check for the
correct number of args as part of the test for whether this can be
special-cased, and if not, fall through to the general code in the main
body of the function to handle that format and issue any warnings.

This makes the code a lot simpler. It also now detects the redundant arg
in printf("%.0f",1,2).

The code is now also more efficient - it tries to check for things like
pat[0] == '%' only once, rather than re-checking for every special-case
variant its trying.

6 years agoPerl_sv_vcatpvfn_flags: simpler redundant arg test
David Mitchell [Thu, 1 Jun 2017 10:55:47 +0000 (11:55 +0100)]
Perl_sv_vcatpvfn_flags: simpler redundant arg test

5.24.0 added a new warning:

    Redundant argument in printf at ....

That warning is issued if there are more args than format elements.
However, it may also warn for invalid format - e.g. for something like
printf("%Z%d", 1,2) you get both

    Invalid conversion in printf: "%Z" at ...
    Redundant argument in printf at ...

Personally I think once once part of the format has been determined to be
invalid, its hard for perl to second-guess in what way the format was
invalid, and thus to be able to conclude that there is in fact a redundant
arg.

So this commit commit suppresses any "redundant" warning once an "invalid"
warning has been issued.

Doing this makes it possible to simplify the code and remove the
used_explicit_ix variable.

Apart from warnings, used_explicit_ix was only used in %p to check for
'simple' special forms - but that code checks for a trailing '$' character
anyway, so that test was redundant.

6 years agoPerl_sv_vcatpvfn_flags: fix comment typo
David Mitchell [Thu, 1 Jun 2017 10:29:35 +0000 (11:29 +0100)]
Perl_sv_vcatpvfn_flags: fix comment typo

6 years agoPerl_sv_vcatpvfn_flags: add comment about wrap
David Mitchell [Thu, 1 Jun 2017 10:27:20 +0000 (11:27 +0100)]
Perl_sv_vcatpvfn_flags: add comment about wrap

6 years agoPerl_sv_vcatpvfn_flags: only do utf8 in radix code
David Mitchell [Thu, 1 Jun 2017 10:08:27 +0000 (11:08 +0100)]
Perl_sv_vcatpvfn_flags: only do utf8 in radix code

For floating point formats, the output can only be utf8 if the radix point
is utf8. Currently the radix point code sets the is_utf8 variable, then
later, in the main floating-point code path, it tests is_utf8 and
upgrades the output string to utf8.

Instead, just do the upgrade directly in the radix code block.

6 years agoPerl_sv_vcatpvfn_flags: simplify radix len adding
David Mitchell [Thu, 1 Jun 2017 10:00:26 +0000 (11:00 +0100)]
Perl_sv_vcatpvfn_flags: simplify radix len adding

Assume the length of the radix point is a constant 1 (i.e. length('.'))
and only increment float_need further if we're in a locale.

6 years agosprintf %a/%A more sanity checks
David Mitchell [Thu, 1 Jun 2017 09:52:12 +0000 (10:52 +0100)]
sprintf %a/%A more sanity checks

For the code which generates hexadecimal floating-point formats,
add extra sanity checks against buffer overruns.

6 years agoS_hextract(): fix #if indentation
David Mitchell [Thu, 1 Jun 2017 09:32:36 +0000 (10:32 +0100)]
S_hextract(): fix #if indentation

a complex set of nested #if/#else/#endif's had incorrect and confusing
indentation.

whitespace-only change

6 years agoPerl_sv_vcatpvfn_flags: simplify some wrap checks
David Mitchell [Wed, 31 May 2017 11:35:34 +0000 (12:35 +0100)]
Perl_sv_vcatpvfn_flags: simplify some wrap checks

Skip doing some overflow checks when we know it can't overflow.

6 years agoPerl_sv_vcatpvfn_flags: simplify float_need calc
David Mitchell [Wed, 31 May 2017 10:59:48 +0000 (11:59 +0100)]
Perl_sv_vcatpvfn_flags: simplify float_need calc

Include another constant addition in the initial assignment, to eliminate
a later wrap check.

6 years agoS_format_hexfp(): s/int/STRLEN/
David Mitchell [Wed, 31 May 2017 10:15:15 +0000 (11:15 +0100)]
S_format_hexfp(): s/int/STRLEN/

In the helper function that sprintf's %a/%A hex floating point values,
the calculation of the number of zeros to pad with should be in terms of
STRLEN rather than int.

A bit academic unless someone ever tries to print a hex f/p value with a
precision > 2Gb digits.

6 years agoop/infnam.t: skip unportable tests
David Mitchell [Wed, 31 May 2017 08:47:27 +0000 (09:47 +0100)]
op/infnam.t: skip unportable tests

sprintf size modifiers L and q aren't available on all platform sizes,
so skip them.

6 years agoPerl_sv_vcatpvfn_flags: add inits to silence gcc
David Mitchell [Tue, 30 May 2017 15:11:37 +0000 (16:11 +0100)]
Perl_sv_vcatpvfn_flags: add inits to silence gcc

Add a couple of unnecessary variable initialisers, to keep gcc's "this
variable might be used uninitialised - then again it might not - in fact I
don't really know what I'm talking about, but I've decided to annoy you
with it anyway" warning at bay.

6 years agoPerl_sv_vcatpvfn_flags: avoid wrap on precision
David Mitchell [Tue, 30 May 2017 14:55:29 +0000 (15:55 +0100)]
Perl_sv_vcatpvfn_flags: avoid wrap on precision

Where the precision is specified literally in the format string,
the integer precision value could wrap. Instead, make it croak with

    Integer overflow in format string

As in other recent commits, the upper limit is set at 1/4 of STRLEN.

6 years agoPerl_sv_vcatpvfn_flags: s/int/STRLEN/g
David Mitchell [Tue, 30 May 2017 14:27:00 +0000 (15:27 +0100)]
Perl_sv_vcatpvfn_flags: s/int/STRLEN/g

There wee a few residual places that used int loop counters, e.g. to
prepend N '0's to a number. Since the N's are of type STRLEN, make the
loop counters STRLEN too.

Its a bit academic since you're unlikely to have a number needing >2Gb
worth of zero padding, but it makes things consistent and easier to audit.

At this point I believe that any remaining usage of int / I32 / U32 in
Perl_sv_vcatpvfn_flags() is legitimate.

6 years agoPerl_sv_vcatpvfn_flags: %n: avoid wrap
David Mitchell [Tue, 30 May 2017 14:11:24 +0000 (15:11 +0100)]
Perl_sv_vcatpvfn_flags: %n: avoid wrap

Its a bit academic, but in principle if a string was longer than 2Gb
chars, the length as set by %n could wrap. So use the correct type(s).

6 years agoPerl_sv_vcatpvfn_flags: width/precis arg wrap
David Mitchell [Tue, 30 May 2017 12:45:35 +0000 (13:45 +0100)]
Perl_sv_vcatpvfn_flags: width/precis arg wrap

When the width or precision is specified via an argument rather than
literally, check whether the value wraps.

Formerly, something like

    $w = 0x100000005;
    printf "%*s", $w, "abc";

might print "  abc" or similar, depending on platform.

Now it croaks with "Integer overflow in format string".

I did wonder whether it should just warn instead, but:

1) over-large literal widths/precisions already croak.
2) Code that has wild field specifiers like that is already likely
   to crash with an out-of-memory error.
3) At least this croak is trappable via eval - OOM isn't.

I also set the maximum allowed value to be 1/4 of the size of a pointer,
to give a safety margin for possible wrapping later

6 years agoPerl_sv_vcatpvfn_flags: move vector initialisation
David Mitchell [Mon, 29 May 2017 16:06:06 +0000 (17:06 +0100)]
Perl_sv_vcatpvfn_flags: move vector initialisation

Move the generation of vecstr/veclen/vec_utf8 into the
vector-initialisation block, rather than being part of the general
'get next arg' block.

Also, stop vecsv being in scope for the whole of the loop block, and make
it two separate tightly-scope vars (with different purposes).

6 years agoPerl_sv_vcatpvfn_flags: warn on missing %v arg
David Mitchell [Mon, 29 May 2017 15:53:06 +0000 (16:53 +0100)]
Perl_sv_vcatpvfn_flags: warn on missing %v arg

The explicit arg variant, e.g. %3$vd, didn't give 'missing arg' warning.

6 years agoPerl_sv_vcatpvfn_flags: warn on missing width arg
David Mitchell [Mon, 29 May 2017 15:20:17 +0000 (16:20 +0100)]
Perl_sv_vcatpvfn_flags: warn on missing width arg

It didn't used to warn when the width value was obtained from the next or
specified arg, and there wasn't such an arg.

6 years agoEliminate FETCH_VCATPVFN_ARGUMENT macro
David Mitchell [Mon, 29 May 2017 15:11:01 +0000 (16:11 +0100)]
Eliminate FETCH_VCATPVFN_ARGUMENT macro

This can be simplified so much now that it might as well just be expanded
in situ for its 3 uses.

6 years agoPerl_sv_vcatpvfn_flags: re-indent block
David Mitchell [Mon, 29 May 2017 15:01:26 +0000 (16:01 +0100)]
Perl_sv_vcatpvfn_flags: re-indent block

whitespace-only

6 years agoPerl_sv_vcatpvfn_flags: unify %v vers obj handling
David Mitchell [Mon, 29 May 2017 14:27:18 +0000 (15:27 +0100)]
Perl_sv_vcatpvfn_flags: unify %v vers obj handling

Cureently sv_vcatpvfn_flags() has special handling of the arg under %v
when the arg is a version object, but only via the perlish interface
(argsv and svmax). This commit extends that handling to the C-sih
interface (args).

There seems no good reason not to, and it simplifies the code.

6 years agoPerl_sv_vcatpvfn_flags: unify args handling
David Mitchell [Mon, 29 May 2017 12:49:42 +0000 (13:49 +0100)]
Perl_sv_vcatpvfn_flags: unify args handling

Several places do something along the lines of:

    if (explicit arg index)
        FETCH_VCATPVFN_ARGUMENT(...., svargs[ix-1])
    else
        FETCH_VCATPVFN_ARGUMENT(...., svargs[svix++])

For each of these, reduce the duplicate code by changing the above to
(approximately)

    ix = ix ? ix - 1 : svix++;
    FETCH_VCATPVFN_ARGUMENT(...., svargs[ix])

6 years agosv_vcatpvfn() family: make svmax arg Size_t
David Mitchell [Mon, 29 May 2017 10:16:49 +0000 (11:16 +0100)]
sv_vcatpvfn() family: make svmax arg Size_t

It was formerly I32. It should be unsigned since you can't have a negative
number of args. And although you're unlikely to call sprintf with more
than 0x7fffffff args, it makes it more consistent with other APIs which
we've been gradually expanding to 64-bit/ptrsize. It also makes the
code internal to Perl_sv_vcatpvfn_flags more consistent, when
dealing with explict arg index formats like "%10$s". This function still
has a mix of STRLEN (for string lengths) and Size_t (for arg indexes)
but they are aliases for each other.

I made Perl_do_sprintf()'s len arg SSize_t rather than Size_t, since
it typically gets called with ptr diff arithmetic. Not sure if this is
being overly cautious.

6 years agoS_expect_number(): return STRLEN not I32
David Mitchell [Mon, 29 May 2017 08:59:16 +0000 (09:59 +0100)]
S_expect_number(): return STRLEN not I32

This static function is used by Perl_sv_vcatpvfn_flags() to read in
a width or explicit argument number. It currently returns an I32 result
(and croaks if the number exceeds the maximum possible I32 value).

Change it to return STRLEN, and to croak on the value being greater than
max(STRLEN) / 4.

This doesn't make a lot of difference in practice, since no code is ever
going to be able to successfully create a formatted string that large
without running out of memory anyway. But by making it unsigned and of the
same type used elsewhere in sv_vcatpvfn_flags(), it simplifies auditing
the code for possible wrapping/truncating etc.

The change in the limit where it croaks with "Integer overflow in format
string" has changed as follows:

                     previously                   now
    32-bit system    0x7fffffff            0x3fffffff
    32/64bit system  0x7fffffff            0x3fffffff
    64bit system     0x7fffffff    0x3fffffffffffffff

Setting the limit as 1/4 max rather than 1/2 max is just a safety
net to help avoid wraps/overflows elsewhere.

6 years agoPerl_sv_vcatpvfn_flags: simplify 'c' var
David Mitchell [Sun, 28 May 2017 17:07:14 +0000 (18:07 +0100)]
Perl_sv_vcatpvfn_flags: simplify 'c' var

Make it so that its now *always* the format type ('s', 'd' etc).
Don';t bother initialising it, and *don't* use as as a temporary
buffer (eptr = &c), so it can be stored in a register.

6 years agoPerl_sv_vcatpvfn_flags: reduce scope of 'iv' var
David Mitchell [Sun, 28 May 2017 16:59:47 +0000 (17:59 +0100)]
Perl_sv_vcatpvfn_flags: reduce scope of 'iv' var

6 years agoPerl_sv_vcatpvfn_flags: eliminate 'epix' var
David Mitchell [Sun, 28 May 2017 16:52:25 +0000 (17:52 +0100)]
Perl_sv_vcatpvfn_flags: eliminate 'epix' var

Or rather, reduce its scope to a small block and rename to 'ix'.

6 years agoS_expect_number() re-indent code
David Mitchell [Sun, 28 May 2017 16:49:10 +0000 (17:49 +0100)]
S_expect_number() re-indent code

.. following previous commit. Whitespace only.

6 years agosprintf: move 1..9 test out of S_expect_number()
David Mitchell [Sun, 28 May 2017 16:43:36 +0000 (17:43 +0100)]
sprintf: move 1..9 test out of S_expect_number()

Currently Perl_sv_vcatpvfn_flags() does several checks for "is the next
part of the format a number starting with a '1'..'9'?" It does this by
calling S_expect_number(), which returns 0 if not, or the value of the
number otherwise. For a simple format specifier, this results in multiple
fruitless calls to S_expect_number.

This commits makes it that the caller of S_expect_number is responsible
for checking for the presence of 1..9.

6 years agoPerl_sv_vcatpvfn_flags: more %v optimisation
David Mitchell [Fri, 26 May 2017 23:57:47 +0000 (00:57 +0100)]
Perl_sv_vcatpvfn_flags: more %v optimisation

Only do the code for appending the vector separator in the vector branch.
In particular, don't size the SvGROW for dotstrlen outside of %v.
This makes the %v code a bit slower but everything else a bit faster.

6 years agoPerl_sv_vcatpvfn_flags: test for valid %vX once
David Mitchell [Fri, 26 May 2017 23:17:35 +0000 (00:17 +0100)]
Perl_sv_vcatpvfn_flags: test for valid %vX once

Rather than testing for !vectorize in every conversion case which doesn't
support %v, test once for supported types in the if (vectorize) branch.

That way code which doesn't use %v never has to test for it.

6 years agoPerl_sv_vcatpvfn_flags: join two if blocks
David Mitchell [Fri, 26 May 2017 23:07:48 +0000 (00:07 +0100)]
Perl_sv_vcatpvfn_flags: join two if blocks

convert if (x); if (!x); into an single if/else

6 years agoPerl_sv_vcatpvfn_flags: delay vector arg get
David Mitchell [Fri, 26 May 2017 23:00:10 +0000 (00:00 +0100)]
Perl_sv_vcatpvfn_flags: delay vector arg get

Move the block of code which retrieves the SV which the %v will iterate
over, from just before the /* SIZE */ block to just after. Since that
block doesn't do anything with args or svargs, this should make no
functional difference - but it will allow the next commit to coalesce
if (x); if (!x); into an single if/else.

Apart from cutting and pasting the code block, no other changes have been
made to it.

6 years agoPerl_sv_vcatpvfn_flags: eliminate VECTORIZE_ARGS
David Mitchell [Fri, 26 May 2017 22:49:58 +0000 (23:49 +0100)]
Perl_sv_vcatpvfn_flags: eliminate VECTORIZE_ARGS

This macro is only used once. Just expand it.

6 years agoPerl_sv_vcatpvfn_flags: eliminate ewix local var
David Mitchell [Fri, 26 May 2017 22:42:07 +0000 (23:42 +0100)]
Perl_sv_vcatpvfn_flags: eliminate ewix local var

It's now only used within one code block.

6 years agoPerl_sv_vcatpvfn_flags: remove 'asterisk' var
David Mitchell [Fri, 26 May 2017 22:34:25 +0000 (23:34 +0100)]
Perl_sv_vcatpvfn_flags: remove 'asterisk' var

There was only one remaining use of this local var: in %p, to distinguish
between explicit and implicit width specifier, e.g. %*p or %1$p, vs %2p.
This can be done by just checking whether the char before the p was a '*'
or '$'.

6 years agoPerl_sv_vcatpvfn_flags: further simplify %v logic
David Mitchell [Fri, 26 May 2017 22:20:22 +0000 (23:20 +0100)]
Perl_sv_vcatpvfn_flags: further simplify %v logic

For the common case with no * or v, there now are only 2 test-and-branch
(! '*', ! 'v') rather than 3 (! '*', ! 'v', !asterisk)

This works by putting the *v handling code in the * branch

6 years agoPerl_sv_vcatpvfn_flags: eliminate evix local var
David Mitchell [Fri, 26 May 2017 21:45:02 +0000 (22:45 +0100)]
Perl_sv_vcatpvfn_flags: eliminate evix local var

6 years agoPerl_sv_vcatpvfn_flags: simplify v/asterisk code
David Mitchell [Fri, 26 May 2017 21:26:58 +0000 (22:26 +0100)]
Perl_sv_vcatpvfn_flags: simplify v/asterisk code

The previous commit's rearrangement of the v and * code now allows us to:
1) eliminate the 'vectorarg' bool variable, which is set but no longer
   used;
2) join two adjacent "if (asterisk)" and "if (!asterisk)" blocks into a
   single if/else.

6 years agoPerl_sv_vcatpvfn_flags: move %*v handling earlier
David Mitchell [Fri, 26 May 2017 21:19:44 +0000 (22:19 +0100)]
Perl_sv_vcatpvfn_flags: move %*v handling earlier

Where the v flag appears, and it has non-default separator, i.e.
*v or *NNN$v, retrieve the next or NNNth arg (which defines the separator)
earlier - as soon as we encounter the v flag. This should in theory make
no functional difference since no args are processed between those two
points (so no chance of us stealing something else's arg).

Doing it ealrier makes the conditions simpler (we don't have to check for
(vectorize && vectorarg) later).

The whole code block has been moved as-is with no changes apart from
whitespace.

6 years agoPerl_sv_vcatpvfn_flags: move Inf handling for ints
David Mitchell [Fri, 26 May 2017 17:19:11 +0000 (18:19 +0100)]
Perl_sv_vcatpvfn_flags: move Inf handling for ints

integer-like format types handle Inf/Nan specially. Currently the code to
handle this in the main execution path, guarded by

    if (strchr("BbcDdiOouUXx", c)) ...

After the previous few commits reorganised the int-arg getting code, this
block can now be moved into an int-only section, so not slowing down
other format types.

There should be no functional changes.

I've added some comments to the %c branch explaining why its a special
case.

6 years agoPerl_sv_vcatpvfn_flags: unify int arg fetching
David Mitchell [Fri, 26 May 2017 16:23:09 +0000 (17:23 +0100)]
Perl_sv_vcatpvfn_flags: unify int arg fetching

There are two big blocks of code that do signed and unsigned 'get next int
arg' processing. Combine them (sort of).

Previously it was a bit like

    case 'd':
    case 'i':
        base = 10;
        if (vectorize)
            uv = ...
        else if (arg)
            iv = ...
        else
            iv = SvIV_nomg(argsv);
        if (!vectorize)
            uv = f(iv) for some f.
        goto integer;

    case 'x' base = 16; goto uns_integer;
    case 'u' base = 10; goto uns_integer;
    ...
    uns_integer:
        if (vectorize)
            uv = ...
        else if (arg)
            uv = ...
        else
            uv = SvUV_nomg(argsv);

    integer:
        ... do stuff with base and uv ...

Now it's more like

    case 'd': base = -10; goto get_int_arg_val;
    case 'i': base = -10; goto get_int_arg_val;
    case 'x': base =  16; goto get_int_arg_val;
    case 'u': base =  10; goto get_int_arg_val;

    get_int_arg_val:

        if (vectorize)
            uv = ...
        else if (base < 0) {
            /* signed int type */
            base = -base;
            if (arg)
                iv = ...
            else
                iv = SvIV_nomg(argsv);
            uv = f(iv) for some f.
        }
        else {
            /* unsigned int type */
            if (arg)
                uv = ...
            else
                uv = SvUV_nomg(argsv);
        }

    integer:
        ... do stuff with base and uv ...

Note that in particular the vectorize block of code is no longer
duplicated. This will also allow the next commit to handle Inf/overload
just after the 'get_int_arg_val' label rather than doing it before the
main switch and slowing down the non-integer format types.

Should be no functional changes

6 years agoPerl_sv_vcatpvfn_flags: move %c handling to ints
David Mitchell [Fri, 26 May 2017 15:39:30 +0000 (16:39 +0100)]
Perl_sv_vcatpvfn_flags: move %c handling to ints

%c is in some ways like integer formats - we treat the arg as an integer
(with '0+' overloading and Inf/Nan handling), but then at the end convert
it into a 1 char string rather than sequence of 0..9's.

Move the %c code partially into the main integer handling block of
code; this will shortly allow us to unify the SV-as-integer handling code.

6 years agoPerl_sv_vcatpvfn_flags: %p and Inf/Nan
David Mitchell [Fri, 26 May 2017 15:05:18 +0000 (16:05 +0100)]
Perl_sv_vcatpvfn_flags: %p and Inf/Nan

sprintf("%p", 0+Inf) should print the address of an SV, not the literal
string "Inf". Ditto NaN.

Similarly, sprintf("%p", $x) should print the address of the $x SV,
not triggering a tie fetch or overload method call, nor using the address
of any SV returned by such calls.

6 years agoPerl_sv_vcatpvfn_flags: make 'fill' var a boolean
David Mitchell [Thu, 25 May 2017 11:09:52 +0000 (12:09 +0100)]
Perl_sv_vcatpvfn_flags: make 'fill' var a boolean

Currently the 'fill' local variable is a char, but it only ever holds the
values ' ' or '0'. Make it into a boolean flag instead.

6 years agoPerl_sv_vcatpvfn_flags: do %p specials in %p case
David Mitchell [Thu, 25 May 2017 10:56:44 +0000 (11:56 +0100)]
Perl_sv_vcatpvfn_flags: do %p specials in %p case

There are currently a few special-cased %p variants (but only when called
from C, not from perl) such as %-p, %2p etc. Currently these are handled
specially at the top of main format-element loop, which penalises every
format type. Instead move the handling into the "case 'p'" branch of the
main switch. Which seems more logical, as well as more efficient.

I've also heavily rewritten the big comment block about all the special %p
formats.

6 years agoPerl_sv_vcatpvfn_flags: move UTF8f handling code
David Mitchell [Thu, 25 May 2017 09:29:04 +0000 (10:29 +0100)]
Perl_sv_vcatpvfn_flags: move UTF8f handling code

The special UTF8f format (which is usually defined as something like
"%d%lu%4p") is currently handled as a special case at the top of the main
format-element loop.

Instead move it into the "case "'d'" branch so that it doesn't slow down
everything.

6 years agoPerl_sv_vcatpvfn_flags: add %n code comment
David Mitchell [Wed, 24 May 2017 15:29:16 +0000 (16:29 +0100)]
Perl_sv_vcatpvfn_flags: add %n code comment

point out thngs like "%-4.5n" don't currently warn

6 years agoPerl_sv_vcatpvfn_flags: make %n missing arg fatal
David Mitchell [Wed, 24 May 2017 15:09:25 +0000 (16:09 +0100)]
Perl_sv_vcatpvfn_flags: make %n missing arg fatal

Normally sprintf et al just warn if there aren't enough args; but since %n
wants to write the current string length to the next arg, make it fatal.

Formerly it would croak anyway, but with a spurious "Modification of a
read-only value" error as it as it tried to set &PL_sv_no

6 years agoPerl_sv_vcatpvfn_flags: comment %n deficiency
David Mitchell [Wed, 24 May 2017 14:58:06 +0000 (15:58 +0100)]
Perl_sv_vcatpvfn_flags: comment %n deficiency

This should be fixed sometime:

    /* XXX if sv was originally non-utf8 with a char in the
     * range 0x80-0xff, then if it got upgraded, we should
     * calculate char len rather than byte len here */

6 years agoPerl_sv_vcatpvfn_flags: skip IN_LC(LC_NUMERIC)
David Mitchell [Sat, 20 May 2017 15:01:26 +0000 (16:01 +0100)]
Perl_sv_vcatpvfn_flags: skip IN_LC(LC_NUMERIC)

In a couple of places it does

    if (PL_numeric_radix_sv && IN_LC(LC_NUMERIC)) { ... }

But PL_numeric_radix_sv is set to NULL unless we have a non-standard
radix point (i.e. not "."), and this can only happen when we're in the
scope of 'use locale'. So the IN_LC() should be a redundant (and
expensive) test. Replace it with an assert.

6 years agoPerl_sv_vcatpvfn_flags: set locale at most once
David Mitchell [Sat, 20 May 2017 14:51:31 +0000 (15:51 +0100)]
Perl_sv_vcatpvfn_flags: set locale at most once

Calls to external snprintf-ish functions or that directly access
PL_numeric_radix_sv are supposed to sandwich this access within

    STORE_LC_NUMERIC_SET_TO_NEEDED();
    ....
    RESTORE_LC_NUMERIC();

The code in Perl_sv_vcatpvfn_flags() seems to have gotten a bit confused
as to whether its trying to only set STORE_LC_NUMERIC_SET_TO_NEEDED()
once, then handle one of more %[aefh] format elements, then only
restore on exit. There is code at the end of the function which says:

    RESTORE_LC_NUMERIC();   /* Done outside loop, so don't have to save/restore
                               each iteration. */

but in practice various places within this function (and its helper
function S_format_hexfp() inconsistently repeatedly do
STORE_LC_NUMERIC_SET_TO_NEEDED(); and sometime do RESTORE_LC_NUMERIC().

This commit changes it so that STORE_LC_NUMERIC_SET_TO_NEEDED() is called
at most once, the first time a % format involving a radix point is
encountered, and does RESTORE_LC_NUMERIC(); exactly once at the end of the
function.

Note that while calling STORE_LC_NUMERIC_SET_TO_NEEDED() multiple times
is harmless, its quite expensive, as each time it has to check whether
it's in the scope of 'use locale'. RESTORE_LC_NUMERIC() is cheap if
STORE_LC_NUMERIC_SET_TO_NEEDED() earlier determined that there was nothing
to do.

6 years agoPerl_sv_vcatpvfn_flags: remove redundant code
David Mitchell [Sat, 20 May 2017 12:01:02 +0000 (13:01 +0100)]
Perl_sv_vcatpvfn_flags: remove redundant code

At the start of the function, it marks the output as being utf8 if the
first arg is utf8. But this should be taken care of when the individual
args (including the first one are processed). So its redundant code.

In fact it would sometimes cause the resultant string to be unnecessarily
upgraded to utf8, e.g.:

    my $precis = "9";
    utf8::upgrade($precis);
    my $s = sprintf "%.*f\n", $precis, 1.1;
    # whoops, $s is now utf8

6 years agoPerl_sv_vcatpvfn_flags: remove "%.Ng" special-case
David Mitchell [Sat, 20 May 2017 11:07:23 +0000 (12:07 +0100)]
Perl_sv_vcatpvfn_flags: remove "%.Ng" special-case

This function has special-case handling for the formats "%.0f" and
"%.NNg", to speed things up. This special-casing appears twice,
once near the top of the function for where the format matches exactly
"%.0f" or "%.Ng" (N is 1..99), and once again in the main loop of the
function, where it handles those format elements embedded in the larger
format: "....%.0f..." and "....%.Ng..." (N > 0).

The problem with the "%.Ng" code is that it isn't as robust as the more
general "....%.Ng..." code - in particular the latter checks for a
locale-dependent radix-point when determining needed buffer size.

This commit removes the "%.Ng" special-cased code but leaves the
"....%.Ng..." special-cased code. It makes the former about 7% slower
compared to the situation at the start of this branch. (Part of the effort
in this branch has been to make the "....%.Ng..." code faster, so that
there's less of an overall performance hit by removing "%.Ng").

6 years agoPerl_sv_vcatpvfn_flags: handle %.NNNg case earlier
David Mitchell [Fri, 19 May 2017 15:15:31 +0000 (16:15 +0100)]
Perl_sv_vcatpvfn_flags: handle %.NNNg case earlier

In the main loop, we look for %.NNNg and handle it specially.
Change it so that the special-case is only used when precis is small
enough to that it fits in the local ebuf[] rather than the malloced
PL_efloatbuf. This allows the check for this special case to be done
earlier with less redundant calculations.

6 years agoPerl_sv_vcatpvfn_flags: use quick concat for %.0f
David Mitchell [Fri, 19 May 2017 14:45:51 +0000 (15:45 +0100)]
Perl_sv_vcatpvfn_flags: use quick concat for %.0f

Most floating-point formats now use the quick concat path. But the
"%.0f" shortcut was accidentally bypassing that path. This commit fixes
that.

6 years agoPerl_sv_vcatpvfn_flags: simplify concat of f/p str
David Mitchell [Thu, 18 May 2017 11:47:51 +0000 (12:47 +0100)]
Perl_sv_vcatpvfn_flags: simplify concat of f/p str

Since floating-point formats do their own formatting and padding, skip the
block of code at the end of the main loop which handles appending eptr to
sv, and do our own stripped-down version.

6 years agoPerl_sv_vcatpvfn_flags: s/gconverts/Gconvert's/
David Mitchell [Thu, 18 May 2017 10:44:17 +0000 (11:44 +0100)]
Perl_sv_vcatpvfn_flags: s/gconverts/Gconvert's/

fix a comment, so that a search for the word 'Gconvert' gets a match.
So that a later comment 'See earlier comment about buggy Gconvert' makes
sense.

6 years agoPerl_sv_vcatpvfn_flags: tighten hexfp var scope
David Mitchell [Thu, 18 May 2017 10:32:27 +0000 (11:32 +0100)]
Perl_sv_vcatpvfn_flags: tighten hexfp var scope

Only have the 'hexfp' var declared within the innermost scope it is
actually needed for.

6 years agoPerl_sv_vcatpvfn_flags: rename 'is_simple' var
David Mitchell [Thu, 18 May 2017 10:17:32 +0000 (11:17 +0100)]
Perl_sv_vcatpvfn_flags: rename 'is_simple' var

the definition of 'simple' required the format to have a precision.

6 years agoPerl_sv_vcatpvfn_flags: move pod closer
David Mitchell [Thu, 18 May 2017 10:03:28 +0000 (11:03 +0100)]
Perl_sv_vcatpvfn_flags: move pod closer

Several static functions etc had been added between the pod and the
main function. Move the pod to be just above it.

Also incorporate a comment into the pod about utf8ness of pattern and SV
needing to match.

6 years agoPerl_sv_vcatpvfn_flags: eliminate utf8buf[] var
David Mitchell [Thu, 18 May 2017 09:45:56 +0000 (10:45 +0100)]
Perl_sv_vcatpvfn_flags: eliminate utf8buf[] var

%c for a >255 char generates its utf8 byte representation and stores it in
thiis temporarly buffer:

    U8 utf8buf[UTF8_MAXBYTES+1]

But we already have another temporary buffer, ebuf, for creating floating
point strings, which is big enough. So use that instead.

6 years agoPerl_sv_vcatpvfn_flags: reorganise loop vars
David Mitchell [Thu, 18 May 2017 09:37:42 +0000 (10:37 +0100)]
Perl_sv_vcatpvfn_flags: reorganise loop vars

There are a big chunk of local vars declared at the top of the main loop.
Reorder the declarations to group similar vars together, and add a comment
to each var explaining what its for.

No functional changes.

6 years agoPerl_sv_vcatpvfn_flags: move vars to inner scope
David Mitchell [Thu, 18 May 2017 08:49:08 +0000 (09:49 +0100)]
Perl_sv_vcatpvfn_flags: move vars to inner scope

Add a new scope around the floating-point code, then move some
locals var declarations into that scope.

6 years agoPerl_sv_vcatpvfn_flags: extract hex f/p code
David Mitchell [Thu, 18 May 2017 08:41:15 +0000 (09:41 +0100)]
Perl_sv_vcatpvfn_flags: extract hex f/p code

There is a large block of code (nearly 300 lines) in
Perl_sv_vcatpvfn_flags(), which handles the %a/%A hexadecimal
floating-point format. Move it into new static function,
S_format_hexfp().

No functional changes.