[I created this RT on Sunday but it did not get CC-ed to list or newsgroup. So I'm copying it here.]

Each of these three configuration step packages' runstep()
subroutines calls Parrot::Configure::Data::get() on the options part
of the Parrot::Configure object and provides file-scoped global
variable @args as the argument to get().

@args = qw(verbose without-gmp);

sub runstep {
my ( $self, $conf ) = @_;

my ( $verbose, $without ) = $conf->options->get(@args);
...
}

This differs from other configuration step packages where the
arguments to get() are spelled out and do not depend on how @args has
been populated. So in these three packages, runstep() depends on a
global variable not provided in the subroutine's @_.

This is not a major problem in and of itself. However, patches which
I will soon submit may entail changes in @args and how it is
handled. It would therefore be advisable to not use @args as the
argument to get() in these three packages. Rather, we should simply
spell out the arguments as we do in the other 50+ configuration step
packages.

my ( $verbose, $without ) = $conf->options->get( qw|
verbose
without-gmp
| );

runstep() would henceforth no longer depend on a global variable.

The patch attached implements these revisions. There is no change in
functionality and no change in test results.

If no one objects, I will apply this patch to trunk in 2-3 days.

Thank you very much.
kid51
Index: config/auto/gdbm.pm
===================================================================
--- config/auto/gdbm.pm (revision 21672)
+++ config/auto/gdbm.pm (working copy)
@@ -30,7 +30,10 @@
 sub runstep {
     my ( $self, $conf ) = @_;
 
-    my ( $verbose, $without ) = $conf->options->get(@args);
+    my ( $verbose, $without ) = $conf->options->get( qw|
+        verbose
+        without-gmp
+    | );
 
     if ($without) {
         $conf->data->set( has_gdbm => 0 );
Index: config/auto/gmp.pm
===================================================================
--- config/auto/gmp.pm  (revision 21672)
+++ config/auto/gmp.pm  (working copy)
@@ -29,7 +29,10 @@
 sub runstep {
     my ( $self, $conf ) = @_;
 
-    my ( $verbose, $without ) = $conf->options->get(@args);
+    my ( $verbose, $without ) = $conf->options->get( qw|
+        verbose
+        without-gmp
+    | );
 
     if ($without) {
         $conf->data->set( has_gmp => 0 );
Index: config/gen/icu.pm
===================================================================
--- config/gen/icu.pm   (revision 21672)
+++ config/gen/icu.pm   (working copy)
@@ -31,7 +31,15 @@
 sub runstep {
     my ( $self, $conf ) = @_;
 
-    my ( $verbose, $icushared, $icuheaders, $icuconfig, $without ) = 
$conf->options->get(@args);
+    my ( $verbose, $icushared, $icuheaders, $icuconfig, $without ) =
+        $conf->options->get( qw|
+            verbose
+            icushared
+            icuheaders
+            icu-config
+            without-icu
+        |
+    );
 
     my @icu_headers = qw(ucnv.h utypes.h uchar.h);
     my $autodetect  = !defined($icushared)

Reply via email to