Eliminating "used only once" warnings from List::Util::reduce

[davido]

This is something that's bothered me for awhile now, but until a few days ago I didn't take the time to investigate the "internals" involved.

Consider the following code:

use warnings;
use List::Util;
my $reduced = List::Util::reduce { $a + $b } 1, 2, 3, 4;
print "$reduced\n";

This will generate the following output:

Name "main::a" used only once: possible typo at mytest.pl line 3.
Name "main::b" used only once: possible typo at mytest.pl line 3.
10

On the other hand...

use warnings;
my @sorted = sort { $a <=> $b } 1, 2, 3, 4;
print "@sorted\n";

...will produce...

1 2 3 4

Given that List::Util is a core module, reduce really ought to have the same privileges as sort with respect to gracefully using $a and $b. I looked at what makes sort's code different from reduce, aside from the obvious fact that one sorts and one reduces. I began by trying to find two needles in the Perl code-base haystack (searching for a and b doesn't really narrow it down). tye was helpful in pointing me to some code in op.c.

This seems to be the relevant segment:

STATIC void
S_simplify_sort(pTHX_ OP *o)
{
    dVAR;
    OP *kid = cLISTOPo->op_first->op_sibling;    /* get past pushmark 
+*/
    OP *k;
    int descending;
    GV *gv;
    const char *gvname;
    bool have_scopeop;

    PERL_ARGS_ASSERT_SIMPLIFY_SORT;

    if (!(o->op_flags & OPf_STACKED))
    return;
    GvMULTI_on(gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV));
    GvMULTI_on(gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV));
    ...

GvMULTI_on must have the effect of tricking Perl into thinking that $a and $b were already used more than once, which takes care of the warning. perlguts, says this in reference to to the SV*  get_sv("package::varname", GV_ADD); function:

There are additional macros whose values may be bitwise OR'ed with the GV_ADD argument to enable certain extra features. Those bits are:

GV_ADDMULTI
Marks the variable as multiply defined, thus preventing the:
  Name <varname> used only once: possible typo
warning.

I thought maybe just using GV_ADD|GV_ADDMULTI could have the same effect without an explicit call to the undocumented GvMULTI_on().

Then I looked at the relevant code in List::Utils:

void
reduce(block,...)
    SV * block
PROTOTYPE: &@
CODE:
{
    SV *ret = sv_newmortal();
    int index;
    GV *agv,*bgv,*gv;
    HV *stash;
    SV **args = &PL_stack_base[ax];
    CV* cv    = sv_2cv(block, &stash, &gv, 0);

    if (cv == Nullcv) {
       croak("Not a subroutine reference");
    }

    if(items <= 1) {
    XSRETURN_UNDEF;
    }

    agv = gv_fetchpv("a", GV_ADD, SVt_PV);
    bgv = gv_fetchpv("b", GV_ADD, SVt_PV);
    ...

Ah-hah! Add a call to GvMULTI_on(), or "or"-ing (|) GV_ADDMULTI into the fetch, should yield a patch for reduce that allows it to play nice with warnings.

agv = gv_fetchpv("a", GV_ADD, SVt_PV);
GvMULTI_on(agv); /* No apparent effect. */

agv = gv_fetchpv("a", GV_ADD|GV_ADDMULTI, SVt_PV); /* Nope */
agv = gv_fetchpv("a", GV_ADDMULTI, SVt_PV); /* *sigh* No. */

Which brings me here to the font of Perl wisdom, hoping someone can explain why, why not, and possibly what needs to happen (in the module or perl) to solve it. This would also help to settle the same bug in List::MoreUtils::pairwise, as well as helping me out with a pet project.

Can anyone shed some light?

Update: I'm aware of many of the previous conversations on the topic. I'm looking at this from an internals perspective; what can the XS code of these modules do to eliminate the problem, not how can I make my use of these modules not trigger warnings. Actually, BrowserUk may be on to something... time for more tests. ;)

[tobyink]

If sort BLOCK LIST is encountered in a package, Perl switches off "used only once" warnings for the $a and $b package variables.

use warnings;
use List::Util;
my $reduced = List::Util::reduce { $a + $b } 1, 2, 3, 4;
print "$reduced\n";
()=sort{;}();

... that's why sort is magic.

List::Util could easily eliminate the warning; and it doesn't even need to use XS trickery to do so. It just needs to add something like this to its import method:

my $pkg = caller;
eval qq[ package $pkg; our \$a; our \$b; ];

I believe the old pure Perl implementation of List::Util used to do something along these lines. It seems the shiny, new XS-only version does not. I'd count that as a regression.

[davido]

This is quite helpful, as it identifies why the XS code I referred to works for sort, and not for reduce, and provides a suggestion for how to solve it at the module level. I suspect that it could even still be fixed in the Perl portion of the List::Util module (List/Util.pm) by overriding Exporter::import, and then from within the override, still calling the parent import. The one disadvantage I see in fixing this through import is that it doesn't deal with this:

use warnings;
use List::Util ();
my $reduced = List::Util::reduce { $a + $b } 1,2,3,4;

Import isn't called, and the warning will still occur (untested assertion). The simple retort to that is, well, if you do it that way, you've got to take responsibility for the warning. But I'm thinking that it still might be better just for perl to special-case $a and $b additionally, such that they're exempt from "only once" warnings altogether, just as they're exempt from strict vars.

...just thinking out loud. Thanks for your reply, it was helpful.

[BrowserUk]

Guess: The warning is produced at compile time (of the calling code) and the function isn't seen until runtime?

[davido]

That's a good guess. If you place a "warn" inside of BEGIN, UNITCHECK, CHECK, INIT, and END blocks, you'll see that the "only once" warning comes immediately after the BEGIN, but before UNITCHECK. It's being generated before runtime, so by the time the subroutine is actually called, it's too late.

Those solutions that deal with import are probably the best answer, though it wouldn't fix fully-qualified usages of reduce (where there is no import call).

I think a better solution would simply be for Perl to completely ignore $a and $b with respect to "used only once" warnings. Those should be additionally special-cased. Now to code-dive again and find where that could be implemented.

[BrowserUk]

I think a better solution would simply be for Perl to completely ignore $a and $b with respect to "used only once" warnings.

The biggest problem is: which $s & $b?

That is, it is fairly easy to preempt the warnings for $main::a & $main::b -- by soemthing as simple as BEGIN{ $main::a || $main::b } somewhere in List::Util.pm; -- but if reduce() is called from some other package they won't be the right ones. Ie. If reduce() is called from package fred,then it'll be $fred::a & $fred::b.

And there is no way to determine which package you were called from at BEGIN time. With this added to List::Util.pm:

BEGIN{
    printf "'%s'\n", scalar caller();
    $main::a || $main::b
}

The current package at BEGIN time is main for both of these:

C:\test\primes>perl -Mstrict -MList::Util=reduce -wE" say reduce{ $a +
+$b } 1,2,3"
'main'
6

C:\test\primes>perl -Mstrict -wE"package fred; use List::Util qw[ redu
+ce ]; say reduce{ $a +$b } 1,2,3"
'main'
Name "fred::a" used only once: possible typo at -e line 1.
Name "fred::b" used only once: possible typo at -e line 1.
6

[tobyink]

Yes, the reason it happens after the BEGIN stage rather than during, when most compile-time warnings are issued, is that Perl needs to check the whole lexical scope (i.e. file probably) before it can issue any warnings about variables being used only once. (Because until it's got to the end of the scope it cannot be sure they were only used once.)

[choroba]

See also: Name "main::a" used only once: possible typo.

[Khen1950fx]

This is the type of bug that makes Perl enormously interesting for me. Here's my last attempt:

 
#!/usr/bin/perl -l

use strict;
use warnings;
use List::Util qw/reduce/;

our($a, $b) = @_;
my $reduce = reduce { $a + $b; } 1, 2, 3, 4;
print $reduce;

[davido]

Slick, however the point of the question was missed. If I wanted to solve it in "user code", there are at least a half dozen ways to do it. The question is dealing with how to solve it in either the module's code or perl code.

[BrowserUk]

Just a thought.

If you tweak List::Util so:

use vars qw(@ISA @EXPORT_OK $VERSION $XS_VERSION $TESTING_PERL_ONLY $a
+ $b );
require Exporter;

@ISA        = qw(Exporter);
@EXPORT_OK  = qw(first min max minstr maxstr reduce sum shuffle $a $b 
+);

Then you can import $a & $b so:

C:\test\primes>perl -Mstrict -MList::Util=reduce,$a,$b -wE" say reduce
+{ $a +$b } 1,2,3"
6

C:\test\primes>perl -Mstrict -wE"package fred; use List::Util qw[ redu
+ce $a $b ]; say reduce{ $a +$b } 1,2,3"
6

C:\test\primes

[ikegami]

I wonder if adding the symbols in a call checker associated with reduce would be early enough.

#define PERL_NO_GET_CONTEXT
#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

STATIC OP* ck_reduce(pTHX_ OP* o, GV* namegv, SV* ckobj) {
#define check_reduce(a,b,c) check_reduce(aTHX_ a,b,c)
   PERL_UNUSED_ARG(namegv);
   PERL_UNUSED_ARG(ckobj);
   (void*)gv_fetchpvs("a", GV_ADD|GV_ADDMULTI, SVt_PV));
   (void*)gv_fetchpvs("b", GV_ADD|GV_ADDMULTI, SVt_PV));
   return o;
}

/* ======================================== */

MODULE = List::Util   PACKAGE = List::Util

BOOT:
{
   CV* const reducecv = get_cvn_flags("List::Util::reduce", 18, GV_ADD
+);
   cv_set_call_checker(reducecv, ck_reduce, &PL_sv_undef);
}

There's something missing since $a and $b's package isn't mentioned anywhere, but you should be able to grab the right package since the checker is called for each call of reduce.

If this works, the warning will be disabled if and only if reduce is used, and only for the package using it.