| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The pod/perlmodlib.pod file is generated at build time by pod/perlmodlib.PL,
which looks at the POD of library and pragma files to find "=head1 NAME"
directives. It reads each file a paragraph at a time ($/ = ""), and considers
each individual paragraph as potentially containing POD.
However, if it finds no "=head1 NAME" entry in a module, it silently ignores
it. That's reasonable for dual-life modules — there's often a good reason
why a particular file has no POD — but for single-life modules, this could
well indicate an issue.
In particular, commit effd17dc012719d584aa712c6c7bd5dc142885b6, as part of
rearranging the contents of lib/warnings.pm, inadvertently changed the
context of its "=head1 NAME" line so that it's no longer the start of a
paragraph. This had the effect of removing warnings.pm from perlmodlib; that
gap was pointed out by pink_mist++ on IRC.
The fix for that is trivial: add a blank in the appropriate place in
regen/warnings.pl, and regen.
The change to pod/perlmodlib.PL to die on missing "=head1 NAME" lines has
also yielded a second module (Config::Extensions) that was missing from
perlmodlib for the same reason; that's also fixed in this commit.
|
| |
|
|
|
|
|
|
| |
I benchmarked this line alone in a one-liner and found the sprintf
variant to be roughly 10% faster than the concat version. It’s also
more readable (and maintainable).
(Not significant enough to warrant a perldelta entry.)
|
| |
|
|
|
| |
These were made using the old version number, and the error was not
caught, because of the lateness in getting 5.27.9 tagged
|
| |
|
|
|
|
|
|
|
|
|
| |
The _at_level functions, which have to bypass Carp, were not
reporting non-line-based filehandles correctly. The perl core
does:
..., <fh> chunk 7.
if $/ is not "\n". warnings.pm should do the same. It was using
‘line’.
|
| |
|
|
| |
This will be used in future commits.
|
| |
|
|
| |
since warnings.(pm|pl) was updated in 25ebbc2270
|
| |
|
|
|
|
|
| |
otherwise the new _at_level functions end up including
‘at <handle> line 0’.
[Commit message by the committer.]
|
| |
|
|
| |
This will be used in later commits.
|
| |
|
|
| |
Commit c4583f59133164b3f392c31e9b9573276ec17e74 introduced a pod error.
|
| | |
|
| |
|
|
| |
As proposed in [perl #125330].
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There were some problems arising from some warning bitsets being shorter
than others, which happens when registration of a new warning category
makes new bitsets longer. Most obviously, if a scope used "use warnings
'all'" to turn on all warnings and then turned off some specific warnings,
then that scope wouldn't get warnings for subsequently-registered warning
categories, because its bitset doesn't extend to the bit controlling
the new category. (If just "use warnings 'all'" was used, without any
customisation, then a special hack made that work for new categories.)
It was also possible for a longer bitset to get truncated by a warnings
pragma, because the bitset editing code assumed that all bitsets are
the same length.
To fix this, first the warning bits for the "all" category have to change
meaning. Unlike all other warning categories, the bits for "all" used to
be set only when there were no warning categories disabled; disabling any
would also clear the "all" bits. That was supporting the special hack
mentioned above that the all-warnings bitset work for new categories.
This exception is now removed, so the meaning of the "all" bits is now the
more obvious meaning, of indicating the default treatment that the scope
wants for warnings not falling into any category known to the bitset.
In warnings::warnif() et al, if the caller's bitset is found to be too
short to have a bit for the relevant category, then the setting for the
"all" category is used instead.
Because the length of a bitset is an integral number of bytes, but
only two bits are used per category, the length of a bitset doesn't
precisely indicate which categories had been registered by the time it
was constructed. So the standard bitsets for the "all" category are
now always filled to their byte length, with bits set preemptively for
categories not yet registered that fall within the current bitset length.
When a warnings pragma operates on a bitset, it first expands it to the
preferred length, by duplicating the "all" bits for the categories covered
by the new length. It is careful to maintain the length when combining
the bitset with the standard bitsets for categories. When a bitset is
read from ${^WARNING_BITS} or from caller(), the standard pWARN_ALL
setting is no longer expanded by the core to $warnings::Bits{all},
because the core's short WARN_ALLstring will now be expanded correctly
just like any other bitset.
Fixes [perl #108778].
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
RT #130621
In ckDEAD(), don't check the value of PL_curcop->cop_warnings unless
PL_curcop is non-null.
In the ticket above, the reason that PL_curcop is null is the less
than optimal way that evals free their optree: ideally the optree should
be attached to the eval CV and freed when the CV is; instead a separate
SAVEFREEOP() is done. But that fix is for another time; regardless,
ckDEAD() should have a PL_curcop != NULL guard anyway like isLEXWARN_on()
etc already do.
|
| | |
|
| |
|
|
|
| |
require calls now require ./ to be prepended to the file since . is no
longer guaranteed to be in @INC.
|
| | |
|
| |
|
|
|
|
|
| |
See the explanation in the test added and in the RT ticket.
The solution is to make the warn macros check that PL_curcop
is non-null.
|
| |
|
|
| |
This makes the comment easier to read.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Clang has taken it upon itself to warn when an equality is wrapped in
double parentheses, e.g.
((foo == bar))
Which is a bit dumb, as any code along the lines of
#define isBAR (foo == BAR)
if (isBAR) {}
will trigger the warning.
This commit shuts clang up by putting in a harmless cast:
#define isBAR cBOOL(foo == BAR)
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
• The code previously assumed that any filename basename besides
`strict.pm` meant that the user mistyped `use strict` (e.g. as
`use Strict`). But that could just mean the file was not loaded
from the filesystem, e.g. due to naïve fatpacking.
This is fixed by adding a guard to check that an unexpected value
really is a mis-capitalised variant of `strict.pm`.
• The code previously insisted on either slash or backslash as the
directory separator, which is not strictly portable (though nobody
noticed in years; apparently nobody has tried to run a recent-ish
on a MacOS Classic or RiscOS system).
This is fixed by switching to \b as a best effort, to avoid going
down the rabbit hole of platform-specific separators.
• The code previously used an `unless` statement, declared lexical
variables inside its block, and used ${\EXPR} to interpolate the
__PACKAGE__ constant into the regexp. Each of these increases the
size of the optree, which is only ever executed once, then sticks
around wasting some hundred(s) bytes in almost every single Perl
program in the world.
This is fixed for warnings.pm by rewriting the code with no use of
any temporary variables and single-quoted strings instead of regexp
literals. In strict.pm, we can do even better by moving the code to
the BEGIN block, since BEGIN CVs are freed after running. (We do not
add one to warnings.pm since BEGIN blocks have a creation cost.)
|
| |
|
|
|
|
| |
7e6d00f88633 added the warnif() function and changed most uses of
warnings:enabled() to use warnif(), including this one. Revert
just that part.
|
| |
|
|
|
|
|
| |
regen/warnings.pl's $VERSION was at 1.04 despite it being modified
each time warnings.pm is modified.
So make them use the same version number.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
After additional discussions on perl5-porters and #p5p, no one seems to
be violently objecting to the idea that FATAL warnings need a much
stronger warning about risks and that FATAL => 'all' should actually be
'discouraged' in the official, perlpolicy sense.
The text of this commit has been posted to perl5-porters for discussion
and approved by those who objected to earlier language.
I dare not call it "consensus" for fear of the consequences, but no one
has raised further obstacles to making this change.
|
| |
|
|
|
|
|
| |
...at least for now.
This reverts commits 0d314ba30623b19c36dfc97ac4b6ecb94cb406f4
and ce3778a3796be3e4604ed9b3671ea624c5affe0b.
|
| |
|
|
| |
Slight grammatical touch-up to cautions against FATAL warnings.
|
| |
|
|
|
|
|
|
|
|
|
| |
Discussions on p5p have reached reasonable consensus that fatal warnings are
a misfeature.
This commit marks fatal warning as "discouraged" and moves up the
related cautionary documentation to highlight the risks of using them.
For details on the many historical and current issues with fatal warning, see
http://www.nntp.perl.org/group/perl.perl5.porters/2015/01/msg225235.html
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts & amends my v5.21.7-151-gea5519d and Karl Williamson's
v5.21.7-183-g2f3cbe1, the latter was only need because of the
former.
I've also taken the opportunity to fix the long-standing trivial bug
with misaligned code in warnings.{pm,h}. That was easier to commit along
with this than to split it up from the other generated changes.
Why revert this? See the "use warnings 'absolutely-all-almost';" thread
on perl5-porters for the latest summary:
http://www.nntp.perl.org/group/perl.perl5.porters/2015/01/msg225066.html
Basically as I explained in v5.21.7-151-gea5519d the current design of
the API makes it too contentious to freely add new warnings, but there's
no consensus on how to solve that. I.e. whether we should just add them
to "all", or do this change, or several other possible things outlined
in that thread and elsewhere.
Since the deadline for contentious changes for v5.22 is already past us
I'm backing this out for now.
|
| | |
|
| |
|
|
| |
This is a step in the process of adding that subpragma.
|
| |
|
|
| |
By not indentins verbatim text so much, we don't run over 79 columns.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When someone suggests a new warning on p5p it always often up being
argued about on the basis that it'll break existing code, and that we
shouldn't add warnings for possibly legitimate code just because it's
unusual or odd.
As I pointed out in a discussion about RT #121025 (see [1]) we only keep
having this discussion because until now we've had no facility to add
new warnings outside of the default set that'll be retroactively enabled
for everything that does 'use warnings'. This patch introduces such a
facility.
As a proof of concept I'm adding a warning for something that was added
as a warning in the past, but pulled out because it was deemed too
controversial at the time: warning about the use of grep in void
context.
That warning was added back in v5.10.0-218-g74295f0 but quickly pulled
out in v5.10.0-230-gf5df478. See [2] for the discussion about it at the
time.
Now if you do:
use warnings;
grep /42/, (1,2);
You'll get no warnings as before, but if you do:
use warnings qw(extra); # Or its sole subcategory: void_unusual
grep /42/, (1,2);
You'll get a warning about "Unusual use of grep in void context". To
turn off this warning once you've turned it on it's *not* sufficient to
do:
no warnings;
You need to do:
no warnings qw(pedantic);
Or:
no warnings qw(everything);
I'm willing to change that, but first we should ask ourselves whether
this should continue to remain a symmetric operation:
{use,no} warnings ['all'];
There's more elaboration on how this works in the changes I'm making to
the perldelta and the warnings documentation. But briefly this should be
100% backwards compatible, but allow us to have our cake and eat it too
in the future by adding new warnings without imposing them on existing
code written against older perl versions (unless that code explicitly
requested to get new warnings as they upgrade perl).
The patch to the warnings.pm documentation lays out a backwards
compatibility policy for warnings, we promise that we'll continue the
status quo with the "all" category, but for other categories (including
future additions) we'll make such promises on a per-category basis.
TODO: I wanted to come up with some more general facility for being able
to add these new warnings without altering the behavior of the -w and -W
switches. I.e. now we'll emit this, as intended:
$ ./perl -Ilib -w -e 'grep /42/, (1,2)'
$ ./perl -Ilib -W -e 'grep /42/, (1,2)'
$ ./perl -Ilib -e 'use warnings; grep /42/, (1,2)'
$ ./perl -Ilib -e 'use warnings "extra"; grep /42/, (1,2)'
Unusual use of grep in void context at -e line 1.
I.e. we don't want -w and -W to mean "use warnings 'everything'", it
should continue to mean "use warnings 'all'". But due to how they're
implemented I couldn't find an easy way to generalize this. Right now
I'm just hardcoding an exception to the new warning category I've added
outside "all" for these warnings.
That should be followed-up with a more general solution, but for now if
we only have a few of these catogeries we should be fine.
This patch incorporates work from Andreas Guðmundsson
<andreasg@nasarde.org> who picked up an earlier version of mine and
figured out the change being made to mg.c here. That change removes an
optimization in the ${^WARNING_BITS} magic which might make things a tad
slower.
1. https://rt.perl.org/Ticket/Display.html?id=121025#txn-1276663
2. http://www.nntp.perl.org/group/perl.perl5.porters/2007/12/msg131922.html
|
| |
|
|
|
| |
This category will be used in future commits for warnings that are
entirely because of locale issues.
|
| |
|
|
|
| |
Also correct the description of lvref magic. When it was first added,
it was for list assignments only, but that soon changed.
|
| |
|
|
|
|
| |
Some lines end with spaces, remove that, use tabs instead of spaces in code
so the perl code is less bytes to read from disk. This patch saved 183
bytes. Part of [perl #122955].
|
| |
|
|
|
|
|
|
| |
warnings.pm is the hottest file/takes the most read() calls of any
module during a make all. By moving POD to the end, ~40KB of OS read()
IO was reduced to ~16KB of OS read() IO calls. Also the parser doesn't need
to search for Perl code in the POD further lessining load time because of
the __END__ token. Filed as [perl #122955].
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Implement RT #121025 and add a "redundant" warning category that
currently only warns about redundant arguments to printf. Now similarly
to how we already warned about missing printf arguments:
$ ./miniperl -Ilib -we 'printf "%s\n", qw()'
Missing argument in printf at -e line 1.
We'll now warn about redundant printf arguments:
$ ./miniperl -Ilib -we 'printf "%s\n", qw(x y)'
Redundant argument in printf at -e line 1.
x
The motivation for this is that I recently fixed an insidious
long-standing 6 year old bug in a codebase I maintain that came down to
an issue that would have been detected by this warning.
Things to note about this patch:
* It found a some long-standing redundant printf arguments in our own
ExtUtils::MakeMaker code which I submitted fixes to in
https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/pull/84 and
https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/pull/86,
those fixes were merged into blead in v5.19.8-265-gb33b7ab
* This warning correctly handles format parameter indexes (e.g. "%1$s")
for some value of correctly. See the comment in t/op/sprintf2.t for
an extensive discussion of how I've handled that.
* We do the correct thing in my opinion when a pattern has redundant
arguments *and* an invalid printf format. E.g. for the pattern "%A%s"
with one argument we'll just warn about an invalid format as before,
but with two arguments we'll warn about the invalid format *and* the
redundant argument.
This helps to disambiguate cases where the user just meant to write a
literal "%SOMETHING" v.s. cases where he though "%S" might be a valid
printf format.
* I originally wrote this while the 5.19 series was under way, but Dave
Mitchell has noted that a warning like this should go into blead
after 5.20 is released:
"[...] I think it should go into blead just after 5.20 is
released, rather than now; I think it'd going to kick up a lot of
dust and we'll want to give CPAN module owners maximum lead time
to fix up their code. For example, if its generating warnings in
cpan/ code in blead, then we need those module authors to fix
their code, produce stable new releases, pull them back into
blead, and let them bed in before we start pushing out 5.20 RC
candidates"
I agree, but we could have our cake and eat it too if "use warnings"
didn't turn this on but an explicit "use warnings qw(redundant)" did.
Then in 5.22 we could make "use warnings" also import the "redundant"
category, and in the meantime you could turn this on
explicitly.
There isn't an existing feature for adding that kind of warning in
the core. And my attempts at doing so failed, see commentary in RT
#121025.
The warning needed to be added to a few places in sv.c because the "",
"%s" and "%-p" patterns all bypass the normal printf handling for
optimization purposes. The new warning works correctly on all of
them. See the tests in t/op/sprintf2.t.
It's worth mentioning that both Debian Clang 3.3-16 and GCC 4.8.2-12
warn about this in C code under -Wall:
$ cat redundant.c
#include <stdio.h>
int main(void) {
printf("%d\n", 123, 345);
return 0;
}
$ clang -Wall -o redundant redundant.c
redundant.c:4:25: warning: data argument not used by format string [-Wformat-extra-args]
printf("%d\n", 123, 345);
~~~~~~ ^
1 warning generated.
$ gcc -Wall -o redundant redundant.c
redundant.c: In function ‘main’:
redundant.c:4:5: warning: too many arguments for format [-Wformat-extra-args]
printf("%d\n", 123, 345);
^
So I'm not the first person to think that this might be generally
useful.
There are also other internal functions that could benefit from
missing/redundant warnings, e.g. pack. Neither of these things currently
warn, but should:
$ perl -wE 'say pack "AA", qw(x y z)'
xy
$ perl -wE 'say pack "AAAA", qw(x y z)'
xyz
I'll file a bug for that, and might take a stab at implementing it.
|
| |
|
|
|
|
|
|
|
|
|
| |
Ever since the warning for missing printf arguments was added in
v5.11.2-116-g7baa469 the "missing" warning category has been defined in
terms of the "uninitialized" category, so you couldn't turn it on/off
independently of that.
As discussed in RT #121025 I'm hacking on adding a new "reduntant"
category for too many printf arguments. So add the long-missing
"missing" category in preparation for that for consistency.
|
| | |
|
| | |
|
| |
|
|
| |
(and of regen/warnings.pl)
|
| | |
|
| | |
|
