| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As I noted in v5.21.1-12-g826af13 we have subtle bugs in the test suite
because you can do e.g. like($@, '') now which'll be a passing test even
when we have an error, because $@ =~ // will be true.
I'm just changing t/test.pl to not accept non-Regexp arguments, and
fixing up a bunch of test failures that resulted from that. There might
still be more of these in tests that I'm just not running, I've also
changed some of these from $str =~ /foo/ to $str eq 'foo'
(i.e. s/like/is/) in cases where that appeared to work, but it might
break some systems.
Let's just find that out via the smokers.
|
| |
|
|
|
| |
Fix a silly bug in v5.21.1-12-g826af13, when $Q was false I added 12
extra test, but didn't declare them, do that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The problem with this was that I didn't test this on a system where $Q
was false. On those systems we not only warn about "Invalid conversion"
but also about the missing sprintf argument, as intended.
The test broke because the local __WARN__ handler would clobber the
"conversion" warning with the "missing" warning, the fix is easy, just
accumulate the warnings with ".=".
While poking at this I discovered a bug that's been here ever since the
test was added back in v5.10.0-1461-g53f65a9. If we emitted a warning on
systems where $Q was true we'd pass the test, this is because the empty
regex will match warnings, the test actually meant to use is() not
like(). It's also a bug that t/test.pl accepts non-regexes as the second
argument, Test::More doesn't:
$ perl -MTest::More -wle 'like "", ""'
not ok 1
# Failed test at -e line 1.
# '' doesn't look much like a regex to me.
# Tests were run but no plan was declared and done_testing() was not seen.
This change might cause some additional black smoke because we'll now be
detecting warnings that we previously ignored, but I don't think this'll
fail on systems where $Q is true.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| | |
|
| |
|
|
|
|
|
| |
Add Dmitri Tikhonov to Perl 5 AUTHORS.
Increment IPC/Open3.pm $VERSION.
For: RT #119843, originally reported by A.N.
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This is a follow-up to 12f55bbce575aecc, which fell victim to a
bug workaround. Because the broken pipes on VMS sometimes put
multiple blank lines in test output, we collapse multiple newlines
into one. Which breaks tests that genuinely have multiple blank
lines in the output.
So don't leave the output blank, which coincidentally makes it a
lot easier to see which print statement produces which line of
output.
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
Upstream, but we are adding a platform specific hints file.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
For S_ functions, remove the context.
For Perl_ functions, add PERL_UNUSED_CONTEXT.
Tricky because sometimes depends on DEBUGGING, and sometimes
on whether we are have PERL_IMPLICIT_SYS.
(Why all the mathoms Perl_is_uni_... and Perl_is_utf8_...
functions are not being whined about is a mystery.)
vutil.c (included via util.c) has one of these, but it's cpan/,
and a known problem: https://rt.cpan.org/Ticket/Display.html?id=96100
|
| |
|
|
|
|
| |
This reverts commit 148f39b7de6eae9ddd59e0b0aff691d6abea7aca.
(Still needs more work, but wanted to see how well this passed with Jenkins.)
|
| |
|
|
|
|
| |
Definitely not *after* it. It marks the start of the unreachable,
not the first unrechable line. And if they are in that order,
it looks better to linebreak after the lint hint.
|
| |
|
|
| |
(Worked in smokes, bombed under Jenkins.)
|
| |
|
|
|
|
|
| |
Furthermore, make the GCC_DIAG_IGNORE and _RESTORE to be dNOOPs,
so that they can be at any level of the code, including global, even
when the compiler is not gcc (or lookalike). If they are just empty,
";" will be left at the call site.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This problem turns out to be a misspelling in two places of a compiler
definition. Since the definition didn't exist (as it was misspelled),
the #ifdef failed.
I don't know how really to test this as it is locale collation, which
varies by locale, and we would be relying on vendor-supplied locales
which may be inconsistent between platforms. I intend to tackle
improvements to collaction later this release cycle, and should come up
with tests at that time. The failing tests in the module were comparing
the Perl sort results with those of the module, and finding they differ.
|
| | |
|
| | |
|
| |
|
|
| |
This fixes the GCC_DIAG_IGNORE() + GCC_DIAG_RESTORE with clang.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
| |
Has been AFAICT for years / always, the documentation just
had it wrong in pre-4.0 gccs.
|
| |
|
|
|
| |
For C89 compliancy, this is the most common thinko people make.
While waiting for -pedantic -std=c89, which will catch the same.
|
| | |
|
| |
|
|
| |
"clean" as in "not needed anymore".
|
| | |
|
| |
|
|
|
|
| |
Another possibility is that clang has become smarter,
and that needs to be made dependent on clang version.
But for now, let's opt for simplicity and less logic.
|
