Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add support for new warning categories outside of "all"
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- Loading branch information