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@​perl.org> wrote​:

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

[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@​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@​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.

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@​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.

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@​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@​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@​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.

[p5pRT]

From @demerphq

On 20 January 2014 13​:50, 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?

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@​gmail.com> wrote​:

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

[p5pRT]

From @rjbs

* Ævar Arnfjörð Bjarmason <avarab@​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@​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'