Consistently use size_t as array count/index in OP_MULTIPARAM

[leonerd]

Recently-added code was written largely by copying existing practices in various places. In particular, much of the original code was using UV typed variables to store sizes and indexes in various list or array structures, counts of parameters, and so on.

On fully 64-bit platforms this is all fine, but on 32-bit platforms with -Duse64bitint enabled, this is a 64-bit integer that won't ever be counting that high. These values might as well be stored as 32-bit integers in that case.

• This set of changes does not require a perldelta entry.

[tonycoz]

You need to change from IVuf (mis-used for UV here) to "%zu" for size_t

B.xs:1184:43: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1184 |                 ret = Perl_newSVpvf(aTHX_ "%" IVdf ",%" IVdf,
      |                                           ^~~
 1185 |                                     p->params, p->opt_params);
      |                                     ~~~~~~~~~
      |                                      |
      |                                      size_t {aka unsigned int}
B.xs:1184:43: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1184 |                 ret = Perl_newSVpvf(aTHX_ "%" IVdf ",%" IVdf,
      |                                           ^~~
 1185 |                                     p->params, p->opt_params);
      |                                                ~~~~~~~~~~~~~
      |                                                 |
      |                                                 size_t {aka unsigned int}

[leonerd]

B.xs:1184:43: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Wformat=]

Oh oops. That code deals with the previous generation, struct op_argcheck_aux and not my new multiparam. I didn't intend to change that one. I shall undo the change in op.h

[leonerd]

Fixed that, and also fixed the code in dump.c that prints these, which I had previously forgotten

[tonycoz]

Visible in the i386 CI, which looks related:

APItest.xs:1207:38: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1207 |                 SV *retsv = newSVpvf("multiparam:%" UVuf "..%" UVuf ":%c",
      |                                      ^~~~~~~~~~~~~~
 1208 |                         p->min_args, p->n_positional, p->slurpy ? p->slurpy : '-');
      |                         ~~~~~~~~~~~   
      |                          |
      |                          size_t {aka unsigned int}
../../embed.h:922:69: note: in definition of macro 'newSVpvf'
  922 | #   define newSVpvf(...)                        Perl_newSVpvf(aTHX_ __VA_ARGS__)
      |                                                                     ^~~~~~~~~~~
APItest.xs:1207:38: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1207 |                 SV *retsv = newSVpvf("multiparam:%" UVuf "..%" UVuf ":%c",
      |                                      ^~~~~~~~~~~~~~
 1208 |                         p->min_args, p->n_positional, p->slurpy ? p->slurpy : '-');
      |                                      ~~~~~~~~~~~~~~~
      |                                       |
      |                                       size_t {aka unsigned int}
../../embed.h:922:69: note: in definition of macro 'newSVpvf'
  922 | #   define newSVpvf(...)                        Perl_newSVpvf(aTHX_ __VA_ARGS__)
      |                                                                     ^~~~~~~~~~~
APItest.xs:1212:42: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1212 |                         sv_catpvf(retsv, ":%s=%" UVf, namepv, paramidx);
      |                                          ^~~~~~~              ~~~~~~~~
      |                                                               |
      |                                                               size_t {aka unsigned int}
../../embed.h:923:72: note: in definition of macro 'sv_catpvf'
  923 | #   define sv_catpvf(a,...)                     Perl_sv_catpvf(aTHX_ a,__VA_ARGS__)
      |                                                                        ^~~~~~~~~~~
APItest.xs:1214:42: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1214 |                         sv_catpvf(retsv, ":(anon)=%" UVf, paramidx);
      |                                          ^~~~~~~~~~~      ~~~~~~~~
      |                                                           |
      |                                                           size_t {aka unsigned int}
../../embed.h:923:72: note: in definition of macro 'sv_catpvf'
  923 | #   define sv_catpvf(a,...)                     Perl_sv_catpvf(aTHX_ a,__VA_ARGS__)
      |                                                                        ^~~~~~~~~~~

If you want to test build locally, I use:

./Configure -des -Dusedevel -Dcc='cc -m32' -Dlibs='-lm' -Duse64bitint

on Debian, though you'll need to install some extra packages.

(possibly with -Accflags=-Wformat=error)

[tonycoz]

The change itself is fine, though it looks like the code in XS::APItest and its tests don't handle/test for Unicode signature parameters.

[leonerd]

Mmmm, yes there's not a lot of non-ASCII testing around the code, I find. When I added t/class/utf8.t there wasn't a lot of precedent to copy.