printf warns about too few arguments, but not too many

[p5pRT]

Migrated from rt.perl.org#121025 (status was 'resolved')

Searchable as RT121025$

[p5pRT]

From @avar

Created by @avar

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.  

Perl Info

``` 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 ```

[p5pRT]

From @jkeenan

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.

I believe this would fall into the category of excessive hand-holding. YAGNI.

Thank you very much. Jim Keenan

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @avar

On Sat\, Jan 18\, 2014 at 1​:26 AM\, James E Keenan via RT \perlbug\-followup@​perl\.org wrote​:

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.

[p5pRT]

From @jkeenan

On Fri Jan 17 17​:41​:38 2014\, avarab@​gmail.com wrote​: [snip]

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

[p5pRT]

From zefram@fysh.org

AEvar Arnfjorth Bjarmason wrote​:

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.)

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

[p5pRT]

From @iabyn

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 thought I was wrong once\, but I was mistaken.

[p5pRT]

From @wolfsage

On Fri\, Jan 17\, 2014 at 8​:40 PM\, Ævar Arnfjörð Bjarmason \avarab@​gmail\.com wrote​:

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)

[p5pRT]

From @druud62

On 2014-01-18 21​:16\, Dave Mitchell wrote​:

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 would not warn like this if the format string contains parameter indices\, because any parameter can go unused with those.

-- Ruud

[p5pRT]

From @khwilliamson

On 01/18/2014 01​:16 PM\, Dave Mitchell wrote​:

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

[p5pRT]

From @abigail

On Sat\, Jan 18\, 2014 at 07​:01​:21PM +0000\, Zefram wrote​:

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

[p5pRT]

From @druud62

On 2014-01-18 22​:23\, Dr.Ruud wrote​:

On 2014-01-18 21​:16\, Dave Mitchell wrote​:

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 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

[p5pRT]

From @ap

* Æ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.)

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

[p5pRT]

From @avar

On Sun\, Jan 19\, 2014 at 9​:17 AM\, Aristotle Pagaltzis via RT \perlbug\-followup@&#8203;perl\.org wrote​:

* Ævar Arnfjörð Bjarmason \avarab@&#8203;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.

[p5pRT]

From perl5-porters@perl.org

avar at cpan.org wrote​:

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.

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.

[p5pRT]

From @tonycoz

On Sat\, Jan 18\, 2014 at 08​:16​:37PM +0000\, Dave Mitchell wrote​:

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

[p5pRT]

From @rjbs

* Dave Mitchell \davem@&#8203;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.

-- rjbs

[p5pRT]

From @demerphq

On 20 January 2014 02​:01\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* Dave Mitchell \davem@&#8203;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/"

[p5pRT]

From @abigail

On Mon\, Jan 20\, 2014 at 01​:22​:53PM +0100\, demerphq wrote​:

On 20 January 2014 02​:01\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* Dave Mitchell \davem@&#8203;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.

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.

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

[p5pRT]

From @rjbs

* demerphq \demerphq@&#8203;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?

-- rjbs

[p5pRT]

From @demerphq

On 20 January 2014 13​:43\, Abigail \abigail@&#8203;abigail\.be wrote​:

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/"

[p5pRT]

From @avar

On Mon\, Jan 20\, 2014 at 1​:50 PM\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* demerphq \demerphq@&#8203;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.

[p5pRT]

From @demerphq

On 20 January 2014 13​:50\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* demerphq \demerphq@&#8203;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/"

[p5pRT]

From @demerphq

On 20 January 2014 15​:27\, Ævar Arnfjörð Bjarmason \avarab@&#8203;gmail\.com wrote​:

On Mon\, Jan 20\, 2014 at 1​:50 PM\, Ricardo Signes \perl\.p5p@&#8203;rjbs\.manxome\.org wrote​:

* demerphq \demerphq@&#8203;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/"

[p5pRT]

From @rjbs

* Ævar Arnfjörð Bjarmason \avarab@&#8203;gmail\.com [2014-01-20T09​:27​:40]

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

[p5pRT]

From @pjcj

On Sun\, Jan 19\, 2014 at 09​:57​:15PM +0100\, Ævar Arnfjörð Bjarmason wrote​:

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

[p5pRT]

From @ikegami

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.

[p5pRT]

From @ikegami

On Wed\, Jan 22\, 2014 at 9​:52 AM\, Eric Brine \ikegami@&#8203;adaelis\.com wrote​:

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.

[p5pRT]

From @avar

On lau 18 jan 12​:16​:52 2014\, davem wrote​:

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.

[p5pRT]

@avar - Status changed from 'open' to 'resolved'