Skip Menu |
Report information
Id: 121025
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: avar <avar [at] cpan.org>
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: Wishlist
Type: core
Perl Version: 5.19.6
Fixed In: (no value)



From: avar [...] cpan.org
Date: Fri, 17 Jan 2014 21:17:53 +0000 (UTC)
CC: avar [...] cpan.org
Subject: printf warns about too few arguments, but not too many
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 3.4k
This is a bug report for perl from avar@cpan.org, generated with the help of perlbug 1.39 running under perl 5.19.6. ----------------------------------------------------------------- [Please describe your issue here] For a long time perl has warned about too few arguments in printf: $ perl -wle 'print $]; printf "%s%s", qw(a)' 5.019006 Missing argument in printf at -e line 1. a I've now just fixed a long-standing 6 year old bug in a codebase I maintain that comes down to not warning about unused printf arguments: $ perl -wle 'print $]; printf "%s%s", qw(a b c)' 5.019006 ab It would be very nice if perl were to have a warning for this. Although this might be intentional by alternating the printf format itself using a conditional, it's much more likely that it's a symtom of something that's a logic error. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=wishlist --- Site configuration information for perl 5.19.6: Configured by avar at Fri Nov 29 02:27:38 UTC 2013. Summary of my perl5 (revision 5 version 19 subversion 6) configuration: Platform: osname=linux, osvers=3.2.0-4-amd64, archname=x86_64-linux uname='linux u.nix.is 3.2.0-4-amd64 #1 smp debian 3.2.35-2 x86_64 gnulinux ' config_args='-de -Dprefix=/home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6 -Dusedevel' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.8.2', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector' --- @INC for perl 5.19.6: /home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6/lib/site_perl/5.19.6/x86_64-linux /home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6/lib/site_perl/5.19.6 /home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6/lib/5.19.6/x86_64-linux /home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6/lib/5.19.6 . --- Environment for perl 5.19.6: HOME=/home/avar LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/v-perlbrew/perl5/perlbrew/perls/perl-5.19.6/bin:/home/avar/g/misc-scripts:/home/avar/bin:/usr/local/bin:/usr/bin:/bin:/usr/games PERLDOC=-MPod::Text::Ansi PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Fri Jan 17 13:18:02 2014, avar wrote: Show quoted text
> > This is a bug report for perl from avar@cpan.org, > generated with the help of perlbug 1.39 running under perl 5.19.6. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > For a long time perl has warned about too few arguments in printf: > > $ perl -wle 'print $]; printf "%s%s", qw(a)' > 5.019006 > Missing argument in printf at -e line 1. > a > > I've now just fixed a long-standing 6 year old bug in a codebase I > maintain that comes down to not warning about unused printf arguments: > > $ perl -wle 'print $]; printf "%s%s", qw(a b c)' > 5.019006 > ab > > It would be very nice if perl were to have a warning for > this. Although this might be intentional by alternating the printf > format itself using a conditional, it's much more likely that it's a > symtom of something that's a logic error. >
I am opposed to this proposal. This would impose more complex behavior on Perl 5's printf than on bash's printf or C's printf. I don't see anything in 'man printf' or 'man 3 printf' on either Linux or Darwin that suggests that such a warning would be desirable. Nor do I see why we should go beyond the behavior documented in, say, http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html. I believe this would fall into the category of excessive hand-holding. YAGNI. Thank you very much. Jim Keenan
Date: Sat, 18 Jan 2014 02:40:49 +0100
From: Ævar Arnfjörð Bjarmason <avarab [...] gmail.com>
CC: Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: Aaron Sherman via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 3.1k
On Sat, Jan 18, 2014 at 1:26 AM, James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Fri Jan 17 13:18:02 2014, avar wrote:
>> >> This is a bug report for perl from avar@cpan.org, >> generated with the help of perlbug 1.39 running under perl 5.19.6. >> >> >> ----------------------------------------------------------------- >> [Please describe your issue here] >> >> For a long time perl has warned about too few arguments in printf: >> >> $ perl -wle 'print $]; printf "%s%s", qw(a)' >> 5.019006 >> Missing argument in printf at -e line 1. >> a >> >> I've now just fixed a long-standing 6 year old bug in a codebase I >> maintain that comes down to not warning about unused printf arguments: >> >> $ perl -wle 'print $]; printf "%s%s", qw(a b c)' >> 5.019006 >> ab >> >> It would be very nice if perl were to have a warning for >> this. Although this might be intentional by alternating the printf >> format itself using a conditional, it's much more likely that it's a >> symtom of something that's a logic error. >>
> > I am opposed to this proposal. > > This would impose more complex behavior on Perl 5's printf than on bash's printf or C's printf. I don't see anything in 'man printf' or 'man 3 printf' on either Linux or Darwin that suggests that such a warning would be desirable. Nor do I see why we should go beyond the behavior documented in, say, http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html.
Warnings like these are usually (always?) not documented in the C standard and are left up to compiler implementors. Any non-trivial C printf implementation is going to emit more warnings than those the open group might document in its reference documentation. It's interesting to see what GCC and Clang have done in this case, i.e. implemented the warning I'm suggesting: $ gcc -Wall -o toomany toomany.c ; ./toomany toomany.c: In function ‘main’: toomany.c:4:5: warning: too many arguments for format [-Wformat-extra-args] printf("%d\n", 123, 456); ^ 123 $ clang -Wall -o toomany toomany.c ; ./toomany toomany.c:4:25: warning: data argument not used by format string [-Wformat-extra-args] printf("%d\n", 123, 456); ~~~~~~ ^ 1 warning generated. 123 Of course unlike perl they can do so with static analysis so it might still hold that this would impose undue complexity on Perl's printf implementation. But looking at it this does not seem to be the case. Perl_sv_vcatpvfn_flags knows how many things are passed in on the stack, and keeps track of how many formats its had to parse. So it seems trivial to emit a warning at the end of it when you find that the number of formats you encountered is less than the things passed in on the stack. I have a WIP patch that does just that. It's not ready yet because I need some sleep right about now. But the test suite is already emitting some interesting output from running with this warning, e.g.: Too many arguments. You passed 7 on the stack but your format only used 4 at /home/avar/g/perl/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Any.pm line 646. From code code in MM_Any.pm that indeed passes in 7 printf arguments and only uses 4. So no, I think I might just need this, and it does look quite useful in practice.
RT-Send-CC: perl5-porters [...] perl.org
On Fri Jan 17 17:41:38 2014, avarab@gmail.com wrote: [snip] Show quoted text
> > Warnings like these are usually (always?) not documented in the C > standard and are left up to compiler implementors. Any non-trivial C > printf implementation is going to emit more warnings than those the > open group might document in its reference documentation. > > It's interesting to see what GCC and Clang have done in this case, > i.e. implemented the warning I'm suggesting: > > $ gcc -Wall -o toomany toomany.c ; ./toomany > toomany.c: In function ‘main’: > toomany.c:4:5: warning: too many arguments for format [-Wformat-extra- > args] > printf("%d\n", 123, 456); > ^ > 123 > $ clang -Wall -o toomany toomany.c ; ./toomany > toomany.c:4:25: warning: data argument not used by format string > [-Wformat-extra-args] > printf("%d\n", 123, 456); > ~~~~~~ ^ > 1 warning generated. > 123 >
[snip] Thank you for providing that additional information. I look forward to seeing your patch (though others will probably be better judges of it than I). jimk
To: Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
From: Zefram <zefram [...] fysh.org>
Date: Sat, 18 Jan 2014 19:01:21 +0000
Download (untitled) / with headers
text/plain 1.6k
AEvar Arnfjorth Bjarmason wrote: Show quoted text
>Perl_sv_vcatpvfn_flags knows how many things are passed in on the >stack, and keeps track of how many formats its had to parse.
If it were not so, it would be at risk of using more arguments than were passed, getting them from never never land, and we'd have bug reports about bad behaviour from passing too few Perl arguments. So it's not merely happenstance that this information is already conveniently passed down the implementation layers far enough for us to actually produce a runtime warning. (Obviously it's inherently *possible* to track the number of arguments passed to the Perl-language printf, because Perl stack discipline does track the extent of the argument list.) Show quoted text
> Too many arguments. You passed 7 on the stack but your format only >used 4 at /home/avar/g/perl/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Any.pm >line 646.
Nice. I like this warning. Wording suggestion: "on the stack" is confusing implementation detail and should be omitted from the warning message. Also, it would probably be better as a single sentence, something like "Too many arguments (7) for format string (expecting 4) in printf at ...". It would be even niftier if we could statically analyse printf expressions at compile time to detect the same problems earlier (especially in rarely-executed code). Obviously that can't be done in all cases, but constant format string followed by all-scalar parameter expressions is reasonably common. The big downside is that a sub call as a parameter expression would disable the static analysis, because it's not clear that the sub will always return exactly one item. -zefram
From: Dave Mitchell <davem [...] iabyn.com>
To: Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Date: Sat, 18 Jan 2014 20:16:37 +0000
Download (untitled) / with headers
text/plain 604b
On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote: Show quoted text
> Nice. I like this warning.
Me too. Although 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 thought I was wrong once, but I was mistaken.
From: "Matthew Horsfall (alh)" <wolfsage [...] gmail.com>
CC: Aaron Sherman via RT <perlbug-followup [...] perl.org>, Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: Ævar Arnfjörð Bjarmason <avarab [...] gmail.com>
Date: Sat, 18 Jan 2014 15:58:00 -0500
Download (untitled) / with headers
text/plain 742b
On Fri, Jan 17, 2014 at 8:40 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: Show quoted text
> I have a WIP patch that does just that. It's not ready yet because I > need some sleep right about now. But the test suite is already > emitting some interesting output from running with this warning, e.g.: > > Too many arguments. You passed 7 on the stack but your format only > used 4 at /home/avar/g/perl/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Any.pm > line 646. > > From code code in MM_Any.pm that indeed passes in 7 printf arguments > and only uses 4. > > So no, I think I might just need this, and it does look quite useful > in practice.
Awesome! I would love to see this included as well. Thank you very much. -- Matthew Horsfall (alh)
From: "Dr.Ruud" <rvtol+usenet [...] isolution.nl>
Subject: Re: [perl #121025] printf warns about too few arguments, but not toomany
To: perl5-porters [...] perl.org, Dave Mitchell <davem [...] iabyn.com>,
Date: Sat, 18 Jan 2014 22:23:32 +0100
Download (untitled) / with headers
text/plain 747b
On 2014-01-18 21:16, Dave Mitchell wrote: Show quoted text
> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
Show quoted text
>> Nice. I like this warning.
> > Me too. Although 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 would not warn like this if the format string contains parameter indices, because any parameter can go unused with those. -- Ruud
Date: Sat, 18 Jan 2014 14:27:27 -0700
From: Karl Williamson <public [...] khwilliamson.com>
To: Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Download (untitled) / with headers
text/plain 620b
On 01/18/2014 01:16 PM, Dave Mitchell wrote: Show quoted text
> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
>> Nice. I like this warning.
> > Me too. Although 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. > >
+1
CC: Perl 5 Porters <perl5-porters [...] perl.org>
From: Abigail <abigail [...] abigail.be>
To: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Date: Sat, 18 Jan 2014 23:54:47 +0100
Download (untitled) / with headers
text/plain 791b
On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote: Show quoted text
> > It would be even niftier if we could statically analyse printf expressions > at compile time to detect the same problems earlier (especially in > rarely-executed code). Obviously that can't be done in all cases, but > constant format string followed by all-scalar parameter expressions is > reasonably common. The big downside is that a sub call as a parameter > expression would disable the static analysis, because it's not clear > that the sub will always return exactly one item.
If the constant format followed by all-scalar parameter expressions is commonly enough that statical analysis is enough, one doesn't need any changes in core, and could write a perlcritic plugin instead and have it live on CPAN. Abigail
To: perl5-porters [...] perl.org, perl5-porters [...] perl.org, Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #121025] printf warns about too few arguments, but nottoomany
From: "Dr.Ruud" <rvtol+usenet [...] isolution.nl>
Date: Sun, 19 Jan 2014 00:15:39 +0100
Download (untitled) / with headers
text/plain 946b
On 2014-01-18 22:23, Dr.Ruud wrote: Show quoted text
> On 2014-01-18 21:16, Dave Mitchell wrote:
>> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
Show quoted text
>>> Nice. I like this warning.
>> >> Me too. Although 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 would not warn like this if the format string contains parameter > indices, because any parameter can go unused with those.
perl -wE' say sprintf join("%1\$s", map "%$_\$s", 2..5), "er", "Just anoth"," P","l hack","!",":)"; ' Just another Perl hacker! -- Ruud
Date: Sun, 19 Jan 2014 09:17:14 +0100
From: Aristotle Pagaltzis <pagaltzis [...] gmx.de>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.2k
* Ævar Arnfjörð Bjarmason <avarab@gmail.com> [2014-01-18 02:45]: Show quoted text
> I have a WIP patch that does just that.
Please make sure it has its own category so that it can be silenced specifically. There has been more than one time where I specifically made use of the fact that `printf` will silently accept more parameters than the format string needs, and I don’t want to have that code start throwing warnings at me that I can only squelch with coarse measures. Show quoted text
> Too many arguments. You passed 7 on the stack but your format only > used 4 at /home/avar/g/perl/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Any.pm > line 646.
I cannot find any other two-sentences warning, except this one: Non-octal character '%c'. Resolved as "%s" (Aside: why is that one two sentences? I think it should use parens to follow common style.) The existing warning for missing printf arguments looks like this: $ perl -we 'printf "%s %s %s %s", 1' Missing argument in printf at -e line 1. Missing argument in printf at -e line 1. Missing argument in printf at -e line 1. That’s annoying. Plus I was hoping for precedent for how to include the numbers, which this approach neatly (*cough*) evades. There is no such established style elsewhere either: $ perl -e 'sub x($) {} x(1,1)' Too many arguments for main::x at -e line 1, near "1) " Execution of -e aborted due to compilation errors. I could not find any warning that gave an expected vs received quantity of anything, so the style for it will have to be cobbled together from existing bits and vague trends in perldiag. :-/ So, after skimming a list of messages from perldiag with descriptions removed, it seems to me the following would be closest to the style used in warnings we already have: Too many arguments in printf (expected 4, got 7) at [...] Substitute “sprintf” when that is being called. I don’t know if it’s possible to replace the silly one-for-each warning for missing arguments with this message (substituting “Missing” for “Too many”, to keep with existing style) without breaking too much code, but it would be a clear usability win. (It would be OK for my use if the same warnings category silences both the warnings for too many and too few arguments.) Regards, -- Aristotle Pagaltzis // <http://plasmasturm.org/>
CC: Nicholas Clark <nick [...] ccl4.org>
From: Ævar Arnfjörð Bjarmason <avar [...] cpan.org>
To: Aaron Sherman via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Date: Sun, 19 Jan 2014 21:57:15 +0100
Download (untitled) / with headers
text/plain 3.9k
On Sun, Jan 19, 2014 at 9:17 AM, Aristotle Pagaltzis via RT <perlbug-followup@perl.org> wrote: Show quoted text
> * Ævar Arnfjörð Bjarmason <avarab@gmail.com> [2014-01-18 02:45]:
>> I have a WIP patch that does just that.
> > Please make sure it has its own category so that it can be silenced > specifically. There has been more than one time where I specifically > made use of the fact that `printf` will silently accept more parameters > than the format string needs, and I don’t want to have that code start > throwing warnings at me that I can only squelch with coarse measures. >
>> Too many arguments. You passed 7 on the stack but your format only >> used 4 at /home/avar/g/perl/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Any.pm >> line 646.
> > I cannot find any other two-sentences warning, except this one: > > Non-octal character '%c'. Resolved as "%s" > > (Aside: why is that one two sentences? I think it should use parens to > follow common style.) > > The existing warning for missing printf arguments looks like this: > > $ perl -we 'printf "%s %s %s %s", 1' > Missing argument in printf at -e line 1. > Missing argument in printf at -e line 1. > Missing argument in printf at -e line 1. > > That’s annoying. Plus I was hoping for precedent for how to include the > numbers, which this approach neatly (*cough*) evades. > > There is no such established style elsewhere either: > > $ perl -e 'sub x($) {} x(1,1)' > Too many arguments for main::x at -e line 1, near "1) > " > Execution of -e aborted due to compilation errors. > > I could not find any warning that gave an expected vs received quantity > of anything, so the style for it will have to be cobbled together from > existing bits and vague trends in perldiag. :-/ > > So, after skimming a list of messages from perldiag with descriptions > removed, it seems to me the following would be closest to the style used > in warnings we already have: > > Too many arguments in printf (expected 4, got 7) at [...] > > Substitute “sprintf” when that is being called. > > I don’t know if it’s possible to replace the silly one-for-each warning > for missing arguments with this message (substituting “Missing” for “Too > many”, to keep with existing style) without breaking too much code, but > it would be a clear usability win. > > (It would be OK for my use if the same warnings category silences both > the warnings for too many and too few arguments.)
(I accidentally replied to the wrong mail and my initial reply opened a new bug at RT #121036, could someone with RT powers please close it.) I've pushed the avar/printf-too-many-arguments branch to perl5.git which has a complete patch that implements this, and as far as I can tell addresses all the concerns raised in this thread. Check it out at: http://perl5.git.perl.org/perl.git/commitdiff/c299833c0ba6b2c1a04c7b98df90305d4d72b02d I did some experimentation on splitting up the "all" warning category into "all" and "new" to address the concern Dave Mitchell raised. If that patch worked we could merge this warning in before 5.20.0, since "use warnings" wouldn't turn it on by default, you'd have to explicitly do "use warnings 'new'", or "use warnings 'redundant'" (currently the only thing in "new"). But I haven't gotten that working yet. Here's the WIP patch for that: http://perl5.git.perl.org/perl.git/commitdiff/21781e853aa567a1e7c75462f3912a4ef1c4d7db In the process of implementing this I also made a new "missing" category for the existing "Missing argument in %s" warning: http://perl5.git.perl.org/perl.git/commitdiff/d4e7a8a601922309865854af2ebc7cd826fcd9ac Before it was piggy-backing on the "uninitialized" category. I'm wondering whether I should merge that into blead. It would "break" (for some value of) "no warnings qw(uninitalized)" on printf formats that have too few arguments, i.e. someone that's explicitly silenced that warning in the past, but would otherwise work just like the current behavior in blead. I think it should be merged before 5.20.0.
From: Father Chrysostomos <sprout [...] cpan.org>
To: perl5-porters [...] perl.org
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Date: 19 Jan 2014 22:30:15 -0000
Download (untitled) / with headers
text/plain 532b
avar at cpan.org wrote: Show quoted text
> It would "break" (for some value of) "no warnings qw(uninitalized)" on > printf formats that have too few arguments, i.e. someone that's > explicitly silenced that warning in the past,
That is not nice. Show quoted text
> but would otherwise work > just like the current behavior in blead. I think it should be merged > before 5.20.0.
I would prefer that a change like that be merged as early on in the release cycle as possible (i.e., after 5.20), so that there is plenty of time to test it, possibly revert it, etc.
Date: Mon, 20 Jan 2014 10:03:14 +1100
From: Tony Cook <tony [...] develop-help.com>
To: perl5-porters [...] perl.org
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Download (untitled) / with headers
text/plain 643b
On Sat, Jan 18, 2014 at 08:16:37PM +0000, Dave Mitchell wrote: Show quoted text
> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
> > Nice. I like this warning.
> > Me too. Although 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.
+1 Tony
Date: Sun, 19 Jan 2014 20:01:13 -0500
To: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
CC: Perl 5 Porters <perl5-porters [...] perl.org>
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Download (untitled) / with headers
text/plain 420b
* Dave Mitchell <davem@iabyn.com> [2014-01-18T15:16:37] Show quoted text
> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
> > Nice. I like this warning.
> > Me too. Although I think it should go into blead just after 5.20 is > released, rather than now
I agree happily with the first part and reluctantly with the second. New warnings will cause a slow cascade of dumb failures best sorted out over a longer time. -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

Date: Mon, 20 Jan 2014 13:22:53 +0100
CC: Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Download (untitled) / with headers
text/plain 1.7k
On 20 January 2014 02:01, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote: Show quoted text
> * Dave Mitchell <davem@iabyn.com> [2014-01-18T15:16:37]
>> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
>> > Nice. I like this warning.
>> >> Me too. Although I think it should go into blead just after 5.20 is >> released, rather than now
> > I agree happily with the first part and reluctantly with the second. New > warnings will cause a slow cascade of dumb failures best sorted out over a > longer time.
FWIW I am against this change in general. I think it adds a bad precedent - nothing else in Perl *warns* about incorrect number of arguments. We either simply use the right number where there is a prototype, or we die because the number is not right. This introduces a special case where we would follow neither of our precedents. IMO it breaks one of the key things that to me makes Perl perlish, which is that Perl doesnt care if you pass more arguments than a sub will actually use. There are many cases where this is quite legitimate. And yes, I have written code that deliberately passes a list of parameters to sprintf and relies on sprintf ignoring the arguments that are unused. Larry set up the rules for Perl to be very tolerant of what the user passes in to subs as arguments, I don't think we should change this now. Having said all that I would be ok with enabling it optionally tho, as I can see it adds value, but not with defaulting it to on. BTW, a logical extreme extension of this patch sequence would be to make the following DWIM: print sprintf "%s %d", "foo", 2, "more string"; IOW, sprintf would know we only want two arguments, and only consume two from that list, just as a sub with a prototype of ($$) would do. cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
From: Abigail <abigail [...] abigail.be>
CC: Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: demerphq <demerphq [...] gmail.com>
Date: Mon, 20 Jan 2014 13:43:42 +0100
On Mon, Jan 20, 2014 at 01:22:53PM +0100, demerphq wrote: Show quoted text
> On 20 January 2014 02:01, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote:
> > * Dave Mitchell <davem@iabyn.com> [2014-01-18T15:16:37]
> >> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
> >> > Nice. I like this warning.
> >> > >> Me too. Although I think it should go into blead just after 5.20 is > >> released, rather than now
> > > > I agree happily with the first part and reluctantly with the second. New > > warnings will cause a slow cascade of dumb failures best sorted out over a > > longer time.
> > FWIW I am against this change in general. I think it adds a bad > precedent - nothing else in Perl *warns* about incorrect number of > arguments. We either simply use the right number where there is a > prototype, or we die because the number is not right. This introduces > a special case where we would follow neither of our precedents.
I agree with this. Show quoted text
> IMO it breaks one of the key things that to me makes Perl perlish, > which is that Perl doesnt care if you pass more arguments than a sub > will actually use. There are many cases where this is quite > legitimate. And yes, I have written code that deliberately passes a > list of parameters to sprintf and relies on sprintf ignoring the > arguments that are unused.
Same here. Show quoted text
> Larry set up the rules for Perl to be very tolerant of what the user > passes in to subs as arguments, I don't think we should change this > now. > > Having said all that I would be ok with enabling it optionally tho, as > I can see it adds value, but not with defaulting it to on. > > BTW, a logical extreme extension of this patch sequence would be to > make the following DWIM: > > print sprintf "%s %d", "foo", 2, "more string"; > > IOW, sprintf would know we only want two arguments, and only consume > two from that list, just as a sub with a prototype of ($$) would do.
A sub with a prototype of ($$) doesn't "only consume" two from the list if it's passed more, it actually dies: #!/usr/bin/perl use 5.010; use strict; use warnings; no warnings 'syntax'; sub two_args ($$) { say "two_args: ", $_ [0]; say "two_args: ", $_ [1]; return; } sub many_args { say "many_args: $_" for @_; return; } many_args two_args "foo", "bar", "baz"; __END__ Too many arguments for main::two_args at /tmp/x line 18, near ""baz";" Execution of /tmp/x aborted due to compilation errors. so if you want the sprintf in print sprintf "%s %d", "foo", 2, "more string"; behave as a ($$) prototypes sub, it ought to die. Unless you're suggesting that the behaviour of ($$) is changed as well. It's only ($) that is special cased: #!/usr/bin/perl use 5.010; use strict; use warnings; no warnings 'syntax'; sub one_arg ($) { say "one_arg: ", $_ [0]; return; } sub many_args { say "many_args: $_" for @_; return; } many_args one_arg "foo", "bar", "baz"; __END__ one_arg: foo many_args: bar many_args: baz Abigail
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
CC: Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: demerphq <demerphq [...] gmail.com>
Date: Mon, 20 Jan 2014 07:50:06 -0500
Download (untitled) / with headers
text/plain 447b
* demerphq <demerphq@gmail.com> [2014-01-20T07:22:53] Show quoted text
> FWIW I am against this change in general. I think it adds a bad > precedent - nothing else in Perl *warns* about incorrect number of > arguments. We either simply use the right number where there is a > prototype, or we die because the number is not right. This introduces > a special case where we would follow neither of our precedents.
Nothing else other than printf, already? -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

From: demerphq <demerphq [...] gmail.com>
CC: Perl 5 Porters <perl5-porters [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: Abigail <abigail [...] abigail.be>
Date: Mon, 20 Jan 2014 15:15:31 +0100
Download (untitled) / with headers
text/plain 1.1k
On 20 January 2014 13:43, Abigail <abigail@abigail.be> wrote: Show quoted text
> On Mon, Jan 20, 2014 at 01:22:53PM +0100, demerphq wrote: > A sub with a prototype of ($$) doesn't "only consume" two from the list > if it's passed more, it actually dies: > > > #!/usr/bin/perl > > use 5.010; > > use strict; > use warnings; > no warnings 'syntax'; > > sub two_args ($$) { > say "two_args: ", $_ [0]; > say "two_args: ", $_ [1]; > return; > } > > sub many_args { > say "many_args: $_" for @_; > return; > } > > many_args two_args "foo", "bar", "baz"; > > __END__ > Too many arguments for main::two_args at /tmp/x line 18, near ""baz";" > Execution of /tmp/x aborted due to compilation errors. > > > so if you want the sprintf in > > print sprintf "%s %d", "foo", 2, "more string"; > > behave as a ($$) prototypes sub, it ought to die. Unless you're suggesting > that the behaviour of ($$) is changed as well. > > It's only ($) that is special cased:
Doh. Thanks, you are right, I confused the behavior of ($) with ($$). Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Mon, 20 Jan 2014 15:27:40 +0100
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
From: Ævar Arnfjörð Bjarmason <avarab [...] gmail.com>
CC: demerphq <demerphq [...] gmail.com>, Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 3.1k
On Mon, Jan 20, 2014 at 1:50 PM, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote: Show quoted text
> * demerphq <demerphq@gmail.com> [2014-01-20T07:22:53]
>> FWIW I am against this change in general. I think it adds a bad >> precedent - nothing else in Perl *warns* about incorrect number of >> arguments. We either simply use the right number where there is a >> prototype, or we die because the number is not right. This introduces >> a special case where we would follow neither of our precedents.
> > Nothing else other than printf, already?
I'd (obviously) like this warning included. But to reply to Yves and Abigail I think we're basically talking about the wrong thing here. Warnings aren't errors because they're recoverable, so they're code that could be construed as being something you intended to do. We have lots of warnings that are potentially OK, like interpolating undef values, attempts to put comments in qw()" etc. E.g. the perl compiler doesn't know if you meant this to be a comment or not: $ perl -wle 'my @kbd_toprow = qw[ ! @ # $ % ^ & * ( ) { } ]' Possible attempt to put comments in qw() list at -e line 1. I think one of the main problems with warnings now is that you can't add a new warning without impacting existing "use warnings;" statements, i.e. we're doing the equivalent of a C compiler adding new warnings to -Wall, and then making users deal with new warnings on new releases of the compiler. Sometimes this is a good thing, but you get into the value judgement of whether a warning would "mostly" warn about legitimate issues, for some value of "mostly". Personally I think that this particular warning should be on by default, but you're going to have that argument again with any new warning we add. How do we determine whether to turn a warning on by default? What I was trying to (but have currently failed) to address in http://perl5.git.perl.org/perl.git/commitdiff/21781e853aa567a1e7c75462f3912a4ef1c4d7db was that you can't even add new warnings without /also/ turning them on by default. It would be nice to have "all" warnings, but also "extra" warnings. This is what C compilers do, but you very quickly end up in the mess that is: https://stackoverflow.com/questions/11714827/how-to-turn-on-literally-all-of-gccs-warnings I'd like for warnings.pm to change so that it could have "extra", "pedantic" etc. warnings. It would also be very useful to be able to turn this on for entire codebases via an env variable or something like sitecustomize, i.e. make: use warnings; Implicitly mean something like: use warnings (exists $ENV{DEFAULT_WARN_CATEGORIES} ? split /,/, $ENV{DEFAULT_WARN_CATEGORIES} : "all"); Or better yet some global callback mechanism so you could selectively turn this on only for some paths, i.e. your codebase, but not system CPAN libraries. But aside from that I think that rather than engaging in subjective arguments about the possible merits of a warning it would be more useful to queue patches to add new warnings up and merge them early in the development release window. Then we can see how big a deal adding them is in practice via CPAN smoking & other testing, we can always revert them if they turn out to be more useless than not.
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
From: demerphq <demerphq [...] gmail.com>
CC: Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
Date: Mon, 20 Jan 2014 16:09:31 +0100
Download (untitled) / with headers
text/plain 683b
On 20 January 2014 13:50, Ricardo Signes <perl.p5p@rjbs.manxome.org> wrote: Show quoted text
> * demerphq <demerphq@gmail.com> [2014-01-20T07:22:53]
>> FWIW I am against this change in general. I think it adds a bad >> precedent - nothing else in Perl *warns* about incorrect number of >> arguments. We either simply use the right number where there is a >> prototype, or we die because the number is not right. This introduces >> a special case where we would follow neither of our precedents.
> > Nothing else other than printf, already?
That was added without me noticing and is a regression that IMO should not have happened. cheers, Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Mon, 20 Jan 2014 16:11:21 +0100
To: Ævar Arnfjörð Bjarmason <avarab [...] gmail.com>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, Dave Mitchell <davem [...] iabyn.com>, Perl 5 Porters <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 3.5k
On 20 January 2014 15:27, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: Show quoted text
> On Mon, Jan 20, 2014 at 1:50 PM, Ricardo Signes > <perl.p5p@rjbs.manxome.org> wrote:
>> * demerphq <demerphq@gmail.com> [2014-01-20T07:22:53]
>>> FWIW I am against this change in general. I think it adds a bad >>> precedent - nothing else in Perl *warns* about incorrect number of >>> arguments. We either simply use the right number where there is a >>> prototype, or we die because the number is not right. This introduces >>> a special case where we would follow neither of our precedents.
>> >> Nothing else other than printf, already?
> > I'd (obviously) like this warning included. But to reply to Yves and > Abigail I think we're basically talking about the wrong thing here. > > Warnings aren't errors because they're recoverable, so they're code > that could be construed as being something you intended to do. We have > lots of warnings that are potentially OK, like interpolating undef > values, attempts to put comments in qw()" etc. E.g. the perl compiler > doesn't know if you meant this to be a comment or not: > > $ perl -wle 'my @kbd_toprow = qw[ ! @ # $ % ^ & * ( ) { } ]' > Possible attempt to put comments in qw() list at -e line 1. > > I think one of the main problems with warnings now is that you can't > add a new warning without impacting existing "use warnings;" > statements, i.e. we're doing the equivalent of a C compiler adding new > warnings to -Wall, and then making users deal with new warnings on new > releases of the compiler. > > Sometimes this is a good thing, but you get into the value judgement > of whether a warning would "mostly" warn about legitimate issues, for > some value of "mostly". > > Personally I think that this particular warning should be on by > default, but you're going to have that argument again with any new > warning we add. How do we determine whether to turn a warning on by > default? > > What I was trying to (but have currently failed) to address in > http://perl5.git.perl.org/perl.git/commitdiff/21781e853aa567a1e7c75462f3912a4ef1c4d7db > was that you can't even add new warnings without /also/ turning them > on by default. It would be nice to have "all" warnings, but also > "extra" warnings. This is what C compilers do, but you very quickly > end up in the mess that is: > https://stackoverflow.com/questions/11714827/how-to-turn-on-literally-all-of-gccs-warnings > > I'd like for warnings.pm to change so that it could have "extra", > "pedantic" etc. warnings. It would also be very useful to be able to > turn this on for entire codebases via an env variable or something > like sitecustomize, i.e. make: > > use warnings; > > Implicitly mean something like: > > use warnings (exists $ENV{DEFAULT_WARN_CATEGORIES} ? split /,/, > $ENV{DEFAULT_WARN_CATEGORIES} : "all"); > > Or better yet some global callback mechanism so you could selectively > turn this on only for some paths, i.e. your codebase, but not system > CPAN libraries. > > But aside from that I think that rather than engaging in subjective > arguments about the possible merits of a warning it would be more > useful to queue patches to add new warnings up and merge them early in > the development release window. Then we can see how big a deal adding > them is in practice via CPAN smoking & other testing, we can always > revert them if they turn out to be more useless than not.
Useful analysis, thanks. I agree that the core problem here is not the warning you added but rather the general issue. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Mon, 20 Jan 2014 12:16:46 -0500
From: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 434b
* Ævar Arnfjörð Bjarmason <avarab@gmail.com> [2014-01-20T09:27:40] Show quoted text
> I think one of the main problems with warnings now is that you can't > add a new warning without impacting existing "use warnings;" > statements, i.e. we're doing the equivalent of a C compiler adding new > warnings to -Wall, and then making users deal with new warnings on new > releases of the compiler.
I agree with this and the rest of your post. -- rjbs
Download signature.asc
application/pgp-signature 490b

Message body not shown because it is not plain text.

CC: Aaron Sherman via RT <perlbug-followup [...] perl.org>, Nicholas Clark <nick [...] ccl4.org>
From: Paul Johnson <paul [...] pjcj.net>
To: Ævar Arnfjörð Bjarmason <avar [...] cpan.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Date: Mon, 20 Jan 2014 20:34:01 +0100
Download (untitled) / with headers
text/plain 2.1k
On Sun, Jan 19, 2014 at 09:57:15PM +0100, Ævar Arnfjörð Bjarmason wrote: Show quoted text
> I've pushed the avar/printf-too-many-arguments branch to perl5.git > which has a complete patch that implements this, and as far as I can > tell addresses all the concerns raised in this thread. Check it out > at: > > http://perl5.git.perl.org/perl.git/commitdiff/c299833c0ba6b2c1a04c7b98df90305d4d72b02d
In this diff, you write: + # Tests for format parameter indexes. + # + # Deciding what to do about these is a bit tricky, and so is + # "correctly" warning about missing arguments on them. + # + # Should we warn if you supply 4 arguments but only use + # argument 1,3 & 4? Or only if you supply 5 arguments and your + # highest used argument is 4? + # + # For some uses of this printf feature (e.g. i18n systems) + # it's a always a logic error to not print out every provided + # argument, but for some other uses skipping some might be a + # feature (although you could argue that then printf should be + # called as e.g: + # + # printf q[%1$s %3$s], x(), undef, z(); + # + # Instead of: + # + # printf q[%1$s %3$s], x(), y(), z(); + # + # Since calling the (possibly expensive) y() function is + # completely redundant there. + # + # We deal with all these potential problems by ignoring it. If + # the pattern contains any format parameter indexes whatsoever + # we'll never warn about redundant arguments. I think you have made the correct decision here. In the presence of parameter indices there should be no warning. Here is a practical example of parameter indices I have in some production code: state $fmt = { minute => '%3$02d-%2$02d-%1$04d %4$02d:%5$02d', hour => '%3$02d-%2$02d-%1$04d %4$02d:00', day => '%3$02d-%2$02d-%1$04d', month => '%2$02d-%1$04d', year => '%1$04d', }; sprintf $fmt->{$res}, $Y, $M, $D, $h, $m I'd be happy to see your nice comment rewritten to be completely confident in your decision. -- Paul Johnson - paul@pjcj.net http://www.pjcj.net
Date: Wed, 22 Jan 2014 09:52:35 -0500
CC: perl5 porters <perl5-porters [...] perl.org>
From: Eric Brine <ikegami [...] adaelis.com>
To: perlbug-followup <perlbug-followup [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
Download (untitled) / with headers
text/plain 716b
On Fri, Jan 17, 2014 at 7:26 PM, James E Keenan via RT <perlbug-followup@perl.org> wrote:
Show quoted text
> It would be very nice if perl were to have a warning for
> this. Although this might be intentional by alternating the printf
> format itself using a conditional, it's much more likely that it's a
> symtom of something that's a logic error.
>

I am opposed to this proposal.

This would impose more complex behavior on Perl 5's printf than on bash's printf or C's printf.

C's printf tends to segfaults if you give the wrong number of arguments (more or less). I don't think that's a behaviour we want to copy. It should be a compile error in C. I don't think it being a warning in Perl is a bad thing.

To: perlbug-followup <perlbug-followup [...] perl.org>
Subject: Re: [perl #121025] printf warns about too few arguments, but not too many
CC: perl5 porters <perl5-porters [...] perl.org>
From: Eric Brine <ikegami [...] adaelis.com>
Date: Wed, 22 Jan 2014 10:01:20 -0500
Download (untitled) / with headers
text/plain 997b
On Wed, Jan 22, 2014 at 9:52 AM, Eric Brine <ikegami@adaelis.com> wrote:
Show quoted text
On Fri, Jan 17, 2014 at 7:26 PM, James E Keenan via RT <perlbug-followup@perl.org> wrote:
> It would be very nice if perl were to have a warning for
> this. Although this might be intentional by alternating the printf
> format itself using a conditional, it's much more likely that it's a
> symtom of something that's a logic error.
>

I am opposed to this proposal.

This would impose more complex behavior on Perl 5's printf than on bash's printf or C's printf.

C's printf tends to segfaults if you give the wrong number of arguments (more or less). I don't think that's a behaviour we want to copy. It should be a compile error in C. I don't think it being a warning in Perl is a bad thing.

Ah yes, variable formats and positional arguments make it likely the argument lists have more fields than required. Didn't realize there was already a long thread when I replied.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On lau 18 jan 12:16:52 2014, davem wrote: Show quoted text
> On Sat, Jan 18, 2014 at 07:01:21PM +0000, Zefram wrote:
> > Nice. I like this warning.
> > Me too. Although 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've just pushed the patch to split up the "missing" category in 3664866, and the patch to add warnings about redundant printf arguments in 4077a6b. We don't have any support for splitting out "extra" warning as discussed in this thread, a friend of mine picked up my patch series for that in https://github.com/andreasgudmundsson/perl5/commits/avar/printf-too-many-arguments and made it work, but I haven't yet reviewed it in any detail. In any case I don't have a use case for any new warning that isn't part of "all", although depending on how 4077a6b smokes on the CPAN we might want to split it out into such a category.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org