develooper Front page | perl.perl5.porters | Postings from January 2018

Re: [perl #132732] use if - behaviour does not match documentation

Thread Previous | Thread Next
From:
Dan Book
Date:
January 24, 2018 17:20
Subject:
Re: [perl #132732] use if - behaviour does not match documentation
Message ID:
CABMkAVUS3YB+gregiiawaHxU96JQK_bOi_iLmxSg_MKf3Mq6hQ@mail.gmail.com
From the source code, 'no if' appears to do the same thing as 'use if' but
runs unimport instead of import. It's not a negation of the condition. This
is consistent with 'no' and 'use' but the negation is not explained very
well IMO.

-Dan

On Wed, Jan 24, 2018 at 11:02 AM, James E Keenan via RT <
perlbug-followup@perl.org> wrote:

> On Wed, 24 Jan 2018 03:39:01 GMT, jkeenan wrote:
> > On Wed, 24 Jan 2018 00:09:26 GMT, grinnz@gmail.com wrote:
> > > On Tue, Jan 23, 2018 at 6:38 PM, <sisyphus1@optusnet.com.au> wrote:
> > >
> > > >
> > > >
> > > > -----Original Message----- From: James E Keenan via RT
> > > > Sent: Wednesday, January 24, 2018 2:45 AM
> > > > To: OtherRecipients of perl Ticket #132732:
> > > > Cc: perl5-porters@perl.org
> > > > Subject: [perl #132732] use if - behaviour does not match
> > > > documentation
> > > >
> > > > On Wed, 17 Jan 2018 02:57:55 GMT, sisyphus wrote:
> > > >>
> > > >>> Hi,
> > > >>>
> > > >>> The documentation states:
> > > >>>
> > > >>> <quote>
> > > >>> The use of => above provides necessary quoting of MODULE . If you
> > > >>> don't
> > > >>> use
> > > >>> the fat comma (eg you don't have any ARGUMENTS), then you'll need
> > > >>> to
> > > >>> quote
> > > >>> the MODULE.
> > > >>> </quote>
> > > >>>
> > > >>> But the "use of => above provides necessary quoting of MODULE" only
> > > >>> if:
> > > >>> 1) strictures are not enabled
> > > >>> &&
> > > >>> 2) MODULE does not contain any occurrences of "::".
> > > >>>
> > > >>
> > > > ....
> > > >
> > > > 3. The problem appears to be specific to 'use strict "subs";'.  The
> > > >> following DWIM:
> > > >> #####
> > > >> use strict;
> > > >> no strict "subs";
> > > >> use if 1, POSIX => qw(:errno_h :fcntl_h);
> > > >> use if 1, Digest::SHA => qw(sha1);
> > > >> use if 1, Non::Existent, qw(foo);
> > > >> #####
> > > >> use strict;
> > > >> no strict "subs";
> > > >> use if 1, POSIX, qw(:errno_h :fcntl_h);
> > > >> use if 1, Digest::SHA, qw(sha1);
> > > >> use if 1, Non::Existent, qw(foo);
> > > >> #####
> > > >>
> > > >> So the problem is not specific to use of the fat arrow.  Indeed, the
> > > >> use
> > > >> of '=>' in the module's SYNOPSIS and DESCRIPTION is misleading.  So
> > > >> if we
> > > >> revise the documentation, we should use commas rather than fat
> > > >> arrows.
> > > >>
> > > >
> > > > Yes, that's a better appraisal.
> > > > It seems the only time the fat arrow helps is when strict "subs" is
> > > > enabled - as it then allows 'use if 1, POSIX => qw(:errno_h
> > > > :fcntl_h);' to
> > > > work.
> > > >
> > > > So maybe the docs could just not mention the fat comma option, and
> > > > specify
> > > > that the module name needs to be placed in quotes if strict "subs" is
> > > > enabled.
> > > > And then we just wait for someone to point out that you don't
> > > > actually need
> > > > to quote the module name when strict subs are enabled, so long as you
> > > > use
> > > > the fat comma && the module name doesn't contain "::" ;-)
> > > >
> > > > But perhaps we need more than just doc fixes.
> > > >>
> > > >
> > > > Perhaps - though I'd personally be quite happy with just a doc fix.
> > > >
> > > > Cheers,
> > > > Rob
> > >
> > >
> > > I think it would be better to have the docs show the module name in
> > > quotes
> > > every time. Use of the fat comma vs a regular comma is then just a
> > > stylistic choice, and we are showing strict-safe examples as we
> > > should.
> > >
> > > -Dan
> >
> > Sounds good.  I'll prepare a patch tomorrow.
>
> This has proven to be trickier than I anticipated -- though not the
> documentation.
>
> I figured it would be a good idea to add tests for every claim made in the
> documentation.  So I started to add tests to dist/if/t/if.t.  (Got some
> advice on #p5p about this from ilmari, haarg and Abigail.)  I wrote what I
> thought were some plausible tests for the 'no if CONDITION, MODULE =>
> ARGUMENTS;' case only to discover that the 'no' did not appear to have any
> impact.  See patch attached.  When I run this against blead, I get:
>
> #####
> $ cd t;./perl harness -v ../dist/if/t/if.t; cd -
>
> ok 1 - "use if" with a false condition, fake pragma
> ok 2 - "use if" with a false condition and a pragma
> ok 3 - "use if" with a true condition, fake pragma
> ok 4 - "use if" with a true condition and a pragma
> ok 5 - expected error message
> ok 6 - "use if" with open
> ok 7 - Too few args to 'use if' returns <undef>
> ok 8 -   ... and returns correct error
> ok 9 - Too few args to 'no if' returns <undef>
> ok 10 -   ... and returns correct error
> AAA: 1311768467284833424
> ok 11 - Cannot hex
> ok 12 - Cannot oct
> not ok 13 - Cannot hex
> #   Failed test 'Cannot hex'
> #   at t/if.t line 57.
> not ok 14 - Cannot oct
> #   Failed test 'Cannot oct'
> #   at t/if.t line 58.
> BBB: 1311768467284833424
> CCC: 1311768467284833424
> ok 15 - Can hex
> # Looks like you failed 2 tests of 16.
> ok 16 - Can oct
> Dubious, test returned 2 (wstat 512, 0x200)
> Failed 2/16 subtests
>
> Test Summary Report
> -------------------
> ../dist/if/t/if.t (Wstat: 512 Tests: 16 Failed: 2)
>   Failed tests:  13-14
>   Non-zero exit status: 2
> Files=1, Tests=16,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.08 cusr
> 0.00 csys =  0.09 CPU)
> Result: FAIL
> #####
>
> Can anyone advise on 'no if'?
>
> Thank you very much.
>
> --
> James E Keenan (jkeenan@cpan.org)
>
> ---
> via perlbug:  queue: perl5 status: open
> https://rt.perl.org/Ticket/Display.html?id=132732
>
> From 1bd4c7ee474965c923e3d6bc19c0b08bc6053701 Mon Sep 17 00:00:00 2001
> From: James E Keenan <jkeenan@cpan.org>
> Date: Tue, 23 Jan 2018 10:46:32 -0500
> Subject: [PATCH] Clarify documentation for 'if' module.
>
> Add tests for "strict 'subs'" -- but tests for 'no CONDITION' are not
> working.
> ---
>  dist/if/if.pm  | 28 ++++++++++------------
>  dist/if/t/if.t | 73 ++++++++++++++++++++++++++++++
> ++++++++++------------------
>  2 files changed, 63 insertions(+), 38 deletions(-)
>
> diff --git a/dist/if/if.pm b/dist/if/if.pm
> index d1cbd00..edc5a23 100644
> --- a/dist/if/if.pm
> +++ b/dist/if/if.pm
> @@ -1,6 +1,6 @@
>  package if;
>
> -$VERSION = '0.0607';
> +$VERSION = '0.0608';
>
>  sub work {
>    my $method = shift() ? 'import' : 'unimport';
> @@ -29,26 +29,22 @@ if - C<use> a Perl module if a condition holds (also
> can C<no> a module)
>
>  =head1 SYNOPSIS
>
> -  use if CONDITION, MODULE => ARGUMENTS;
> -  no if CONDITION, MODULE => ARGUMENTS;
> +    use if CONDITION, "MODULE", ARGUMENTS;
> +    no  if CONDITION, "MODULE", ARGUMENTS;
>
>  =head1 DESCRIPTION
>
>  The C<if> module is used to conditionally load or unload another module.
>  The construct
>
> -  use if CONDITION, MODULE => ARGUMENTS;
> +    use if CONDITION, "MODULE", ARGUMENTS;
>
> -will load MODULE only if CONDITION evaluates to true.
> -The above statement has no effect unless C<CONDITION> is true.
> -If the CONDITION does evaluate to true, then the above line has
> -the same effect as:
> +will load MODULE only if CONDITION evaluates to true.  The above
> statement has
> +no effect unless C<CONDITION> is true.  (The module name must be quoted
> when
> +C<'use strict "subs";'> is in effect.) If the CONDITION does evaluate to
> true,
> +then the above line has the same effect as:
>
> -  use MODULE ARGUMENTS;
> -
> -The use of C<< => >> above provides necessary quoting of C<MODULE>.
> -If you don't use the fat comma (eg you don't have any ARGUMENTS),
> -then you'll need to quote the MODULE.
> +    use MODULE ARGUMENTS;
>
>  If you wanted ARGUMENTS to be an empty list, i.e. have the effect of:
>
> @@ -63,7 +59,7 @@ exactly this effect, at compile time, with:
>
>  The following line is taken from the testsuite for L<File::Map>:
>
> -  use if $^O ne 'MSWin32', POSIX => qw/setlocale LC_ALL/;
> +    use if $^O ne 'MSWin32', POSIX => qw/setlocale LC_ALL/;
>
>  If run on any operating system other than Windows,
>  this will import the functions C<setlocale> and C<LC_ALL> from L<POSIX>.
> @@ -71,7 +67,7 @@ On Windows it does nothing.
>
>  The following is used to L<deprecate> core modules beyond a certain
> version of Perl:
>
> -  use if $] > 5.016, 'deprecate';
> +    use if $] > 5.016, 'deprecate';
>
>  This line is taken from L<Text::Soundex> 3.04,
>  and marks it as deprecated beyond Perl 5.16.
> @@ -85,7 +81,7 @@ unless you've installed a more recent version of
> L<Text::Soundex> from CPAN.
>
>  You can also specify to NOT use something:
>
> - no if $] ge 5.021_006, warnings => "locale";
> +    no if $] ge 5.021_006, warnings => "locale";
>
>  This warning category was added in the specified Perl version (a
> development
>  release).  Without the C<'if'>, trying to use it in an earlier release
> would
> diff --git a/dist/if/t/if.t b/dist/if/t/if.t
> index 4a2b351..4854b7d 100644
> --- a/dist/if/t/if.t
> +++ b/dist/if/t/if.t
> @@ -1,9 +1,9 @@
>  #!./perl
>
>  use strict;
> -use Test::More tests => 10;
> +use Test::More tests => 16;
>
> -my $v_plus = $] + 1;
> +my $v_plus  = $] + 1;
>  my $v_minus = $] - 1;
>
>  unless (eval 'use open ":std"; 1') {
> @@ -12,29 +12,58 @@ unless (eval 'use open ":std"; 1') {
>    eval 'sub open::foo{}';              # Just in case...
>  }
>
> -no strict;
> +{
> +    no strict;
>
> -is( eval "use if ($v_minus > \$]), strict => 'subs'; \${'f'} = 12", 12,
> -    '"use if" with a false condition, fake pragma');
> -is( eval "use if ($v_minus > \$]), strict => 'refs'; \${'f'} = 12", 12,
> -    '"use if" with a false condition and a pragma');
> +    is( eval "use if ($v_minus > \$]), strict => 'subs'; \${'f'} = 12",
> 12,
> +        '"use if" with a false condition, fake pragma');
> +    is( eval "use if ($v_minus > \$]), strict => 'refs'; \${'f'} = 12",
> 12,
> +        '"use if" with a false condition and a pragma');
>
> -is( eval "use if ($v_plus > \$]), strict => 'subs'; \${'f'} = 12", 12,
> -    '"use if" with a true condition, fake pragma');
> +    is( eval "use if ($v_plus > \$]), strict => 'subs'; \${'f'} = 12", 12,
> +        '"use if" with a true condition, fake pragma');
>
> -is( eval "use if ($v_plus > \$]), strict => 'refs'; \${'f'} = 12", undef,
> -    '"use if" with a true condition and a pragma');
> -like( $@, qr/while "strict refs" in use/, 'expected error message'),
> +    is( eval "use if ($v_plus > \$]), strict => 'refs'; \${'f'} = 12",
> undef,
> +        '"use if" with a true condition and a pragma');
> +    like( $@, qr/while "strict refs" in use/, 'expected error message'),
>
> -# Old version had problems with the module name 'open', which is a
> keyword too
> -# Use 'open' =>, since pre-5.6.0 could interpret differently
> -is( (eval "use if ($v_plus > \$]), 'open' => IN => ':crlf'; 12" || 0), 12,
> -    '"use if" with open');
> +    # Old version had problems with the module name 'open', which is a
> keyword too
> +    # Use 'open' =>, since pre-5.6.0 could interpret differently
> +    is( (eval "use if ($v_plus > \$]), 'open' => IN => ':crlf'; 12" ||
> 0), 12,
> +        '"use if" with open');
>
> -is(eval "use if ($v_plus > \$])", undef,
> -   "Too few args to 'use if' returns <undef>");
> -like($@, qr/Too few arguments to 'use if'/, "  ... and returns correct
> error");
> +    is(eval "use if ($v_plus > \$])", undef,
> +       "Too few args to 'use if' returns <undef>");
> +    like($@, qr/Too few arguments to 'use if'/, "  ... and returns
> correct error");
>
> -is(eval "no if ($v_plus > \$])", undef,
> -   "Too few args to 'no if' returns <undef>");
> -like($@, qr/Too few arguments to 'no if'/, "  ... and returns correct
> error");
> +    is(eval "no if ($v_plus > \$])", undef,
> +       "Too few args to 'no if' returns <undef>");
> +    like($@, qr/Too few arguments to 'no if'/, "  ... and returns correct
> error");
> +}
> +
> +{
> +    note(q|RT 132732: strict 'subs'|);
> +    use strict "subs";
> +
> +    {
> +        eval "use if (0 > 1), q|bigrat|, qw(hex oct);";
> +        ok (! bigrat->can('hex'), "Cannot hex");
> +        ok (! bigrat->can('oct'), "Cannot oct");
> +        print STDERR "AAA: ", hex("0x1234567890123490"),"\n";
> +    }
> +
> +    {
> +        eval "no if (1 > 0), q|bigrat|, qw(hex oct);";
> +        ok (! bigrat->can('hex'), "Cannot hex");
> +        ok (! bigrat->can('oct'), "Cannot oct");
> +        print STDERR "BBB: ", hex("0x1234567890123490"),"\n";
> +    }
> +
> +    {
> +        eval "use if (1 > 0), q|bigrat|, qw(hex oct);";
> +        ok (bigrat->can('hex'), "Can hex");
> +        ok (bigrat->can('oct'), "Can oct");
> +        print STDERR "CCC: ", hex("0x1234567890123490"),"\n";
> +    }
> +
> +}
> --
> 2.7.4
>
>
>

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About