Skip Menu |
Report information
Id: 132732
Status: resolved
Priority: 0/
Queue: perl5

Owner: jkeenan <jkeenan [at] cpan.org>
Requestors: sisyphus <sisyphus1 [at] optusnet.com.au>
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



To: <perlbug [...] perl.org>
Date: Wed, 17 Jan 2018 12:40:15 +1100
From: <sisyphus1 [...] optusnet.com.au>
Subject: use if - behaviour does not match documentation
Download (untitled) / with headers
text/plain 2.5k
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 "::". For example, on MS Windows with 5.27.7 and 5.26.0: ########################################## C:\>perl -Mstrict -le "use if 1, POSIX => qw(DBL_MAX);" C:\>perl -Mstrict -le "use if 1, Digest::SHA => qw(sha1);" Bareword "Digest::SHA" not allowed while "strict subs" in use at -e line 1. Execution of -e aborted due to compilation errors. C:\>perl -Mstrict -le "use if 1, Non::Existent => qw(crap);" Bareword "Non::Existent" not allowed while "strict subs" in use at -e line 1. Execution of -e aborted due to compilation errors. C:\>perl -Mstrict -le "use if 0, Non::Existent => qw(crap);" Bareword "Non::Existent" not allowed while "strict subs" in use at -e line 1. Execution of -e aborted due to compilation errors. C:\> ########################################## But if we quote any MODULE that contains any"::", then all works as expected: ########################################## C:\>perl -Mstrict -le "use if 1, 'Digest::SHA' => qw(sha1);" C:\>perl -Mstrict -le "use if 1, 'Non::Existent' => qw(crap);" Can't locate Non/Existent.pm in @INC (you may need to install the Non::Existent module) (@INC contains: C:/_64/blead-5.27.7/site/lib C:/_64/blead-5.27.7/lib) at C:/_64/blead-5.27.7/lib/if.pm line 15. BEGIN failed--compilation aborted at -e line 1. C:\>perl -Mstrict -le "use if 0, 'Non::Existent' => qw(crap);" C:\> ########################################## The problem goes away if we don't load strict.pm. Also, deferring the loading od strict.pm until after *all* "use if ..." statements have been made removes the problem. If we don't want to mess with this behaviour of if.pm, then I think these caveats involving if.pm should be documented eg: ########################################## --- if.pm_orig 2018-01-17 12:11:48 +1100 +++ if.pm 2018-01-17 12:33:43 +1100 @@ -50,6 +50,10 @@ If you don't use the fat comma (eg you don't have any ARGUMENTS), then you'll need to quote the MODULE. +NOTE: If strictures are enabled and C<MODULE> contains any +occurrences of C<< :: >> then you'll need to quote the MODULE, even +if you have used the fat comma. + If you wanted ARGUMENTS to be an empty list, i.e. have the effect of: use MODULE (); ########################################## Cheers, Rob
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 4.2k
On Wed, 17 Jan 2018 02:57:55 GMT, sisyphus wrote: Show quoted text
> 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 "::". > > For example, on MS Windows with 5.27.7 and 5.26.0: > > ########################################## > C:\>perl -Mstrict -le "use if 1, POSIX => qw(DBL_MAX);" > > C:\>perl -Mstrict -le "use if 1, Digest::SHA => qw(sha1);" > Bareword "Digest::SHA" not allowed while "strict subs" in use at -e line 1. > Execution of -e aborted due to compilation errors. > > C:\>perl -Mstrict -le "use if 1, Non::Existent => qw(crap);" > Bareword "Non::Existent" not allowed while "strict subs" in use at -e line > 1. > Execution of -e aborted due to compilation errors. > > C:\>perl -Mstrict -le "use if 0, Non::Existent => qw(crap);" > Bareword "Non::Existent" not allowed while "strict subs" in use at -e line > 1. > Execution of -e aborted due to compilation errors. > > C:\> > ########################################## > > But if we quote any MODULE that contains any"::", then all works as > expected: > > ########################################## > C:\>perl -Mstrict -le "use if 1, 'Digest::SHA' => qw(sha1);" > > C:\>perl -Mstrict -le "use if 1, 'Non::Existent' => qw(crap);" > Can't locate Non/Existent.pm in @INC (you may need to install the > Non::Existent > module) (@INC contains: C:/_64/blead-5.27.7/site/lib > C:/_64/blead-5.27.7/lib) at > C:/_64/blead-5.27.7/lib/if.pm line 15. > BEGIN failed--compilation aborted at -e line 1. > > C:\>perl -Mstrict -le "use if 0, 'Non::Existent' => qw(crap);" > > C:\> > ########################################## > > The problem goes away if we don't load strict.pm. > Also, deferring the loading od strict.pm until after *all* "use if ..." > statements have been made removes the problem. > > If we don't want to mess with this behaviour of if.pm, then I think these > caveats involving if.pm should be documented eg: > > ########################################## > --- if.pm_orig 2018-01-17 12:11:48 +1100 > +++ if.pm 2018-01-17 12:33:43 +1100 > @@ -50,6 +50,10 @@ > If you don't use the fat comma (eg you don't have any ARGUMENTS), > then you'll need to quote the MODULE. > > +NOTE: If strictures are enabled and C<MODULE> contains any > +occurrences of C<< :: >> then you'll need to quote the MODULE, even > +if you have used the fat comma. > + > If you wanted ARGUMENTS to be an empty list, i.e. have the effect of: > > use MODULE (); > > ########################################## > > Cheers, > Rob
1. I confirm your findings. 2. There are currently 10 unit tests in dist/if/t/if.t, all of which are run under 'no strict;'. If you duplicate those tests and run them under 'use strict;', 3 of the 10 immediately fail: ##### not ok 11 - "use if" with a false condition, fake pragma # Failed test '"use if" with a false condition, fake pragma' # at t/if.t line 47. # got: undef # expected: '12' not ok 12 - "use if" with a false condition and a pragma # Failed test '"use if" with a false condition and a pragma' # at t/if.t line 49. # got: undef # expected: '12' not ok 13 - "use if" with a true condition, fake pragma # Failed test '"use if" with a true condition, fake pragma' # at t/if.t line 52. # got: undef # expected: '12' ##### 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. But perhaps we need more than just doc fixes. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
CC: <perl5-porters [...] perl.org>
To: <perlbug-followup [...] perl.org>
Date: Wed, 24 Jan 2018 10:38:20 +1100
Subject: Re: [perl #132732] use if - behaviour does not match documentation
From: <sisyphus1 [...] optusnet.com.au>
Download (untitled) / with headers
text/plain 1.9k
Show quoted text
-----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
To: sisyphus1 [...] optusnet.com.au
Date: Tue, 23 Jan 2018 19:09:15 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
CC: Dave Mitchell via RT <perlbug-followup [...] perl.org>, Perl5 Porters <perl5-porters [...] perl.org>
From: Dan Book <grinnz [...] gmail.com>
Download (untitled) / with headers
text/plain 2.3k
On Tue, Jan 23, 2018 at 6:38 PM, <sisyphus1@optusnet.com.au> wrote:
Show quoted text


-----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
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.7k
On Wed, 24 Jan 2018 00:09:26 GMT, grinnz@gmail.com wrote: Show quoted text
> 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. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 4.6k
On Wed, 24 Jan 2018 03:39:01 GMT, jkeenan wrote: Show quoted text
> 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)
Subject: 0001-Clarify-documentation-for-if-module.patch
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
From: Dan Book <grinnz [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: Dave Mitchell via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #132732] use if - behaviour does not match documentation
Date: Wed, 24 Jan 2018 12:20:43 -0500
Download (untitled) / with headers
text/plain 12.2k
Download (untitled) / with headers
text/html 16.8k

Message body is not shown because it is too large.

To: perl5-porters [...] perl.org
Subject: Re: [perl #132732] use if - behaviour does not match documentation
From: James E Keenan <jkeenan [...] pobox.com>
Date: Wed, 24 Jan 2018 12:33:29 -0500
Download (untitled) / with headers
text/plain 423b
On 01/24/2018 12:20 PM, Dan Book wrote: Show quoted text
> 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. >
So, would a proper test then be to import specific functions, then use 'no if CONDITION, "MODULE" ARGUMENTS to unimport them? jimk
From: Dan Book <grinnz [...] gmail.com>
Date: Wed, 24 Jan 2018 12:37:32 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
To: James E Keenan <jkeenan [...] pobox.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 649b
On Wed, Jan 24, 2018 at 12:33 PM, James E Keenan <jkeenan@pobox.com> wrote:
Show quoted text
On 01/24/2018 12:20 PM, Dan Book wrote:
 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.


So, would a proper test then be to import specific functions, then use 'no if CONDITION, "MODULE" ARGUMENTS to unimport them?


That should be reasonable, but note that not many modules implement unimport; 'no warnings' is probably the most common usage of 'no'.

-Dan
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Wed, 24 Jan 2018 17:38:14 GMT, grinnz@gmail.com wrote: Show quoted text
> On Wed, Jan 24, 2018 at 12:33 PM, James E Keenan <jkeenan@pobox.com> > wrote: >
> > On 01/24/2018 12:20 PM, Dan Book wrote: > >
> >> 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. > >> > >>
> > So, would a proper test then be to import specific functions, then > > use 'no > > if CONDITION, "MODULE" ARGUMENTS to unimport them? > > > >
> That should be reasonable, but note that not many modules implement > unimport; 'no warnings' is probably the most common usage of 'no'. > > -Dan
Well, the reason I used bigrat in the tests is that it does contain a 'sub unimport'. As does 're'. But there I get even stranger results. Try this out (in the patch): ##### { eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; ok (! re->can('is_regexp'), "Cannot is_regexp"); ok (! re->can('regexp_pattern'), "Cannot regexp_pattern"); ok ( re->can('is_regexp'), "Can is_regexp"); ok ( re->can('regexp_pattern'), "Can regexp_pattern"); } ##### I get: ##### not ok 11 - Cannot is_regexp # Failed test 'Cannot is_regexp' # at t/if.t line 73. not ok 12 - Cannot regexp_pattern # Failed test 'Cannot regexp_pattern' # at t/if.t line 74. ok 13 - Can is_regexp ok 14 - Can regexp_pattern ##### Which I read as, "Whether or not the CONDITION evaluates to true or not, the two functions are imported from package 're'." -- James E Keenan (jkeenan@cpan.org)
Date: Wed, 24 Jan 2018 13:16:24 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
To: Dave Mitchell via RT <perlbug-followup [...] perl.org>
CC: Perl5 Porters <perl5-porters [...] perl.org>
From: Dan Book <grinnz [...] gmail.com>
On Wed, Jan 24, 2018 at 1:09 PM, James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text

As does 're'.  But there I get even stranger results.  Try this out (in the patch):

#####
    {
        eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);";
        ok (! re->can('is_regexp'), "Cannot is_regexp");
        ok (! re->can('regexp_pattern'), "Cannot regexp_pattern");
        ok (  re->can('is_regexp'), "Can    is_regexp");
        ok (  re->can('regexp_pattern'), "Can    regexp_pattern");
    }
#####
I get:
#####
not ok 11 - Cannot is_regexp
#   Failed test 'Cannot is_regexp'
#   at t/if.t line 73.
not ok 12 - Cannot regexp_pattern
#   Failed test 'Cannot regexp_pattern'
#   at t/if.t line 74.
ok 13 - Can    is_regexp
ok 14 - Can    regexp_pattern
#####

Which I read as, "Whether or not the CONDITION evaluates to true or not, the two functions are imported from package 're'."


This does not look like it's testing imports; it's testing whether the is_regexp and regexp_pattern functions exist in re::, which is always true.

-Dan

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Wed, 24 Jan 2018 18:16:40 GMT, grinnz@gmail.com wrote: Show quoted text
> On Wed, Jan 24, 2018 at 1:09 PM, James E Keenan via RT < > perlbug-followup@perl.org> wrote:
> > > > > > As does 're'. But there I get even stranger results. Try this out (in > > the patch): > > > > ##### > > { > > eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; > > ok (! re->can('is_regexp'), "Cannot is_regexp"); > > ok (! re->can('regexp_pattern'), "Cannot regexp_pattern"); > > ok ( re->can('is_regexp'), "Can is_regexp"); > > ok ( re->can('regexp_pattern'), "Can regexp_pattern"); > > } > > ##### > > I get: > > ##### > > not ok 11 - Cannot is_regexp > > # Failed test 'Cannot is_regexp' > > # at t/if.t line 73. > > not ok 12 - Cannot regexp_pattern > > # Failed test 'Cannot regexp_pattern' > > # at t/if.t line 74. > > ok 13 - Can is_regexp > > ok 14 - Can regexp_pattern > > ##### > > > > Which I read as, "Whether or not the CONDITION evaluates to true or not, > > the two functions are imported from package 're'." > > > >
> This does not look like it's testing imports; it's testing whether the > is_regexp and regexp_pattern functions exist in re::, which is always true. > > -Dan
Noted. But please run the attached file. To the best of my understanding of the 'if' documentation, the tests in the third block of the file should pass, i.e., they should catch exceptions and set $@. They are not doing so? What are we missing? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 132732-if.t
Download 132732-if.t
text/plain 1.4k
#!/usr/bin/env perl use strict; use warnings; use Test::More tests => 7; eval "use if (1 < 0), q|re|, qw(is_regexp regexp_pattern);"; { my ($pat, $mods, $obj); $obj = qr/foo/i; local $@; eval { ($pat,$mods) = regexp_pattern($obj); }; like($@, qr/^Undefined subroutine \&main::regexp_pattern called/, "'if' condition evaluated false; function regexp_pattern not imported"); local $@; eval { is_regexp($obj); }; like($@, qr/^Undefined subroutine \&main::is_regexp called/, "'if' condition evaluated false; function is_regexp not imported"); } eval "use if (0 < 1), q|re|, qw(is_regexp regexp_pattern);"; { my ($pat, $mods, $obj); $obj = qr/foo/i; ($pat,$mods) = regexp_pattern($obj); ok(defined $pat, "'if' condition evaluated true; regexp_pattern() imported"); ok(defined $mods, "'if' condition evaluated true; regexp_pattern() imported"); ok(is_regexp($obj), "'if' condition evaluated true; is_regexp imported"); } eval "no if (0 < 1), q|re|, qw(is_regexp regexp_pattern);"; { my ($pat, $mods, $obj); $obj = qr/foo/i; local $@; eval { ($pat,$mods) = regexp_pattern($obj); }; like($@, qr/^Undefined subroutine \&main::regexp_pattern called/, "'no' condition evaluated true; function regexp_pattern unimported"); local $@; eval { is_regexp($obj); }; like($@, qr/^Undefined subroutine \&main::is_regexp called/, "'no' condition evaluated true; function is_regexp unimported"); }
From: Dan Book <grinnz [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: Dave Mitchell via RT <perlbug-followup [...] perl.org>
Date: Wed, 24 Jan 2018 15:17:22 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
Download (untitled) / with headers
text/plain 573b
On Wed, Jan 24, 2018 at 3:12 PM, James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text

Noted.  But please run the attached file.  To the best of my understanding of the 'if' documentation, the tests in the third block of the file should pass, i.e., they should catch exceptions and set $@.  They are not doing so?  What are we missing?

 The 'unimport' method (actually implemented in the 'bits' method) of re.pm does not appear to have any logic for 'unimporting' functions. It only has reverse logic for the 'debug', 'strict', and flag parameters.

-Dan
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 716b
On Wed, 24 Jan 2018 17:38:14 GMT, grinnz@gmail.com wrote: Show quoted text
> > > >
> That should be reasonable, but note that not many modules implement > unimport; 'no warnings' is probably the most common usage of 'no'. > > -Dan
There are many instances of the 'no' function in the source code, but very few instances of the string 'no if'. See attachment for output of 'ack '\bno\s+if\b' .'. Almost every instance is testing one of Perl's special variables against some value and, as you say, most are warnings-related. Unfortunately that isn't getting us any close to solving the problem. Is anyone familiar with where/how 'no' is implemented in the source code? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: no-if-in-core.txt
Download no-if-in-core.txt
text/plain 1.3k
dist/IO/t/IO.t:52: no if $^V >= 5.17.4, warnings => "deprecated"; dist/Devel-PPPort/parts/inc/misc:696: no if $^V > v5.17.9, warnings => "experimental::lexical_topic"; dist/Devel-PPPort/t/misc.t:63: no if $^V > v5.17.9, warnings => "experimental::lexical_topic"; dist/Data-Dumper/t/indent.t:95: no if $] < 5.011, warnings => 'deprecated'; dist/if/Changes:7: - Better failure message for 'no if': It previously always dist/if/Changes:8: said 'use if', even if 'no if' was what was specified (Karl dist/if/if.pm:33: no if CONDITION, MODULE => ARGUMENTS; dist/if/if.pm:88: no if $] ge 5.021_006, warnings => "locale"; dist/if/t/if.t:38:is(eval "no if ($v_plus > \$])", undef, dist/if/t/if.t:39: "Too few args to 'no if' returns <undef>"); dist/if/t/if.t:40:like($@, qr/Too few arguments to 'no if'/, " ... and returns correct error"); cpan/autodie/t/exceptions.t:12:no if $] >= 5.017011, warnings => "experimental::smartmatch"; cpan/autodie/lib/Fatal.pm:1060: no if \$\] >= 5.017011, warnings => "experimental::smartmatch"; pod/perl5180delta.pod:44: no if $] >= 5.018, warnings => "experimental::feature_name"; pod/perl5180delta.pod:408: no if $] >= 5.018, warnings => "experimental::smartmatch"; pod/perl5200delta.pod:1537: no if $] >= 5.01908, warnings => "experimental::autoderef"; t/comp/parser.t:451:eval 'no if $] >= 5.17.4 warnings => "deprecated"';
Date: Wed, 24 Jan 2018 19:53:59 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
To: Dave Mitchell via RT <perlbug-followup [...] perl.org>
CC: Perl5 Porters <perl5-porters [...] perl.org>
From: Dan Book <grinnz [...] gmail.com>
On Wed, Jan 24, 2018 at 4:18 PM, James E Keenan via RT <perlbug-followup@perl.org> wrote:
Show quoted text
On Wed, 24 Jan 2018 17:38:14 GMT, grinnz@gmail.com wrote:
> >
> >
> That should be reasonable, but note that not many modules implement
> unimport; 'no warnings' is probably the most common usage of 'no'.
>
> -Dan

There are many instances of the 'no' function in the source code, but very few instances of the string 'no if'.  See attachment for output of 'ack '\bno\s+if\b' .'.  Almost every instance is testing one of Perl's special variables against some value and, as you say, most are warnings-related.

Unfortunately that isn't getting us any close to solving the problem.  Is anyone familiar with where/how 'no' is implemented in the source code?

 
'no' is the same as 'use' but it calls unimport instead of import. The complications just come from modules not generally implementing an unimport method that mirrors the import (mostly because unimporting functions doesn't really make practical sense). So only pragmas that create lexical effects tend to use it.

-Dan 

CC: <perl5-porters [...] perl.org>
To: <perlbug-followup [...] perl.org>
Subject: Re: [perl #132732] use if - behaviour does not match documentation
Date: Thu, 25 Jan 2018 12:42:10 +1100
From: <sisyphus1 [...] optusnet.com.au>
Download (untitled) / with headers
text/plain 1.9k
Show quoted text
-----Original Message----- From: James E Keenan via RT Sent: Thursday, January 25, 2018 8:18 AM To: OtherRecipients of perl Ticket #132732: Cc: perl5-porters@perl.org Subject: [perl #132732] use if - behaviour does not match documentation
> Is anyone familiar with where/how 'no' is implemented in the source code?
I'm thinking 3 distinct cases: 1) You want to load a module unconditionally ('use MODULE;') 2) You want to load a module conditionally ('use if CONDITION, MODULE, FUNCTIONS;') 3) Having loaded a module, you then wish to conditionally remove specific piece of that module's functionality ('no if CONDITION, MODULE, EXCLUSIONS'). As an example, 'no if' seems to work fine with warnings.pm: ### 1 ### use warnings; no if 0, warnings => "numeric"; print "$]\n"; print "hello world" + 1, "\n"; print "\$z: $z\n"; ######## Outputs: Name "main::z" used only once: possible typo at try.pl line 5. 5.027008 Argument "hello world" isn't numeric in addition (+) at try.pl line 4. 1 Use of uninitialized value $z in concatenation (.) or string at try.pl line 5. $z: If we change the condition to 'no if 1', then the output is the same, except that the numeric warning has disappeared. And that's as to be expected. Similarly, with strict.pm, the following works fine, outputting "DONE": use strict; no if 1, strict => "subs"; use if 1, Math::BigInt => 1; print "DONE\n"; That would blow up at compile time if strict "subs" was enabled - which is exactly what happens if the condition is changed to 'no if 0'. But the same doesn't happen with re, as James has discovered. use re; no if 1, re => "is_regexp"; is_regexp(1); print "DONE\n"; simply outputs "DONE" instead of throwing an error. Is the difference that (wrt re.pm), we're actually explicitly calling a function - whereas, with warnings, "numeric" is not a function that we explicitly call, just as "subs" is not a strict.pm function that we explicitly call ?? Cheers, Rob
From: Dan Book <grinnz [...] gmail.com>
CC: Dave Mitchell via RT <perlbug-followup [...] perl.org>, Perl5 Porters <perl5-porters [...] perl.org>
Date: Wed, 24 Jan 2018 21:00:56 -0500
Subject: Re: [perl #132732] use if - behaviour does not match documentation
To: sisyphus1 [...] optusnet.com.au
Download (untitled) / with headers
text/plain 850b
On Wed, Jan 24, 2018 at 8:42 PM, <sisyphus1@optusnet.com.au> wrote: Show quoted text

Is the difference that (wrt re.pm), we're actually explicitly calling a function - whereas, with warnings, "numeric" is not a function that we explicitly call, just as "subs" is not a strict.pm function that we explicitly call ??

As I explained before, the difference is that the 'unimport' method for re.pm does not unimport subroutines, because that's impractical.

I believe the question was about implementation of 'no' itself, but for use cases, there are also several pragmas that are useful via 'no' even if they weren't already activated with 'use', usually because it reads better that way, for example:

no autovivification;
no bareword::filehandles;

So one could conceive of a case where you'd want to conditionally "activate" one of these pragmas.

-Dan
From: Abigail <abigail [...] abigail.be>
Subject: Re: [perl #132732] use if - behaviour does not match documentation
Date: Thu, 25 Jan 2018 12:33:29 +0100
To: James E Keenan via RT <perlbug-followup [...] perl.org>
CC: "OtherRecipients of perl Ticket #132732": ;, perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.4k
On Wed, Jan 24, 2018 at 10:09:56AM -0800, James E Keenan via RT wrote: Show quoted text
> On Wed, 24 Jan 2018 17:38:14 GMT, grinnz@gmail.com wrote:
> > On Wed, Jan 24, 2018 at 12:33 PM, James E Keenan <jkeenan@pobox.com> > > wrote: > >
> > > On 01/24/2018 12:20 PM, Dan Book wrote: > > >
> > >> 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. > > >> > > >>
> > > So, would a proper test then be to import specific functions, then > > > use 'no > > > if CONDITION, "MODULE" ARGUMENTS to unimport them? > > > > > >
> > That should be reasonable, but note that not many modules implement > > unimport; 'no warnings' is probably the most common usage of 'no'. > > > > -Dan
> > Well, the reason I used bigrat in the tests is that it does contain a 'sub unimport'. > > As does 're'. But there I get even stranger results. Try this out (in the patch): > > ##### > { > eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; > ok (! re->can('is_regexp'), "Cannot is_regexp"); > ok (! re->can('regexp_pattern'), "Cannot regexp_pattern"); > ok ( re->can('is_regexp'), "Can is_regexp"); > ok ( re->can('regexp_pattern'), "Can regexp_pattern"); > } > ##### > I get: > ##### > not ok 11 - Cannot is_regexp > # Failed test 'Cannot is_regexp' > # at t/if.t line 73. > not ok 12 - Cannot regexp_pattern > # Failed test 'Cannot regexp_pattern' > # at t/if.t line 74. > ok 13 - Can is_regexp > ok 14 - Can regexp_pattern > ##### > > Which I read as, "Whether or not the CONDITION evaluates to true or not, the two functions are imported from package 're'." >
That code checks whether the class 're' has a subroutine 'is_regexp' -- which it will as long as it's loaded. To check whether it has been imported, you need to call "can" on the class functions are imported *to*: ##### { eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; ok (! main::->can('is_regexp'), "Cannot is_regexp"); ok (! main::->can('regexp_pattern'), "Cannot regexp_pattern"); ok ( main::->can('is_regexp'), "Can is_regexp"); ok ( main::->can('regexp_pattern'), "Can regexp_pattern"); } ##### Assuming your code runs in the main package. Abigail
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.8k
On Thu, 25 Jan 2018 11:29:38 GMT, abigail@abigail.be wrote: Show quoted text
> On Wed, Jan 24, 2018 at 10:09:56AM -0800, James E Keenan via RT wrote:
> > On Wed, 24 Jan 2018 17:38:14 GMT, grinnz@gmail.com wrote:
> > > On Wed, Jan 24, 2018 at 12:33 PM, James E Keenan > > > <jkeenan@pobox.com> > > > wrote: > > >
> > > > On 01/24/2018 12:20 PM, Dan Book wrote: > > > >
> > > >> 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. > > > >> > > > >>
> > > > So, would a proper test then be to import specific functions, > > > > then > > > > use 'no > > > > if CONDITION, "MODULE" ARGUMENTS to unimport them? > > > > > > > >
> > > That should be reasonable, but note that not many modules implement > > > unimport; 'no warnings' is probably the most common usage of 'no'. > > > > > > -Dan
> > > > Well, the reason I used bigrat in the tests is that it does contain a > > 'sub unimport'. > > > > As does 're'. But there I get even stranger results. Try this out > > (in the patch): > > > > ##### > > { > > eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; > > ok (! re->can('is_regexp'), "Cannot is_regexp"); > > ok (! re->can('regexp_pattern'), "Cannot regexp_pattern"); > > ok ( re->can('is_regexp'), "Can is_regexp"); > > ok ( re->can('regexp_pattern'), "Can regexp_pattern"); > > } > > ##### > > I get: > > ##### > > not ok 11 - Cannot is_regexp > > # Failed test 'Cannot is_regexp' > > # at t/if.t line 73. > > not ok 12 - Cannot regexp_pattern > > # Failed test 'Cannot regexp_pattern' > > # at t/if.t line 74. > > ok 13 - Can is_regexp > > ok 14 - Can regexp_pattern > > ##### > > > > Which I read as, "Whether or not the CONDITION evaluates to true or > > not, the two functions are imported from package 're'." > >
> > > That code checks whether the class 're' has a subroutine 'is_regexp' > -- > which it will as long as it's loaded. > > To check whether it has been imported, you need to call "can" on the > class functions are imported *to*: > > ##### > { > eval "use if (0 > 1), q|re|, qw(is_regexp regexp_pattern);"; > ok (! main::->can('is_regexp'), "Cannot is_regexp"); > ok (! main::->can('regexp_pattern'), "Cannot regexp_pattern"); > ok ( main::->can('is_regexp'), "Can is_regexp"); > ok ( main::->can('regexp_pattern'), "Can regexp_pattern"); > } > ##### > > Assuming your code runs in the main package. > > > > Abigail
Thanks for the suggestion. I believe I now have better documentation and testing of 'if'. Please review the new patch attached, 132732-0001-if-module-clarify-documentation-and-test-more-thorou.patch. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 132732-0001-if-module-clarify-documentation-and-test-more-thorou.patch
From e202bfb954217349f5e61bddbb92ba10021ea242 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] 'if' module: clarify documentation and test more thoroughly. The documentation for 'if' made certain claims about the need to quote or not quote a module name preceding a "fat arrow" ('=>') operator. These claims were shown to be unfounded in most cases when "use strict 'subs'" was in effect. In the course of writing better documentation, it was observed that the "no if" case was very under-tested, poorly documented and hence poorly understood. Hence, more tests have been added and the documentation has been extensively revised. However, there have been no changes in source code or functionality. Make porting/podcheck.t happy. Compensate for functions not available on older perls. For: RT # 132732. --- dist/if/if.pm | 84 ++++++++++++++++++++++-------------------------- dist/if/t/if.t | 100 ++++++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 117 insertions(+), 67 deletions(-) diff --git a/dist/if/if.pm b/dist/if/if.pm index d1cbd00..23ba642 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'; @@ -25,76 +25,70 @@ __END__ =head1 NAME -if - C<use> a Perl module if a condition holds (also can C<no> a module) +if - C<use> a Perl module if a condition holds =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 +=head2 C<use if> - use if CONDITION, MODULE => ARGUMENTS; +The C<if> module is used to conditionally load another module. The construct: -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: + use if CONDITION, "MODULE", ARGUMENTS; - use MODULE ARGUMENTS; +... will load C<MODULE> only if C<CONDITION> evaluates to true; it has no +effect if C<CONDITION> evaluates to false. (The module name, assuming it +contains at least one C<::>, 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: -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: +For example, the F<Unicode::UCD> module's F<charinfo> function will use two functions from F<Unicode::Normalize> only if a certain condition is met: + + use if defined &DynaLoader::boot_DynaLoader, + "Unicode::Normalize" => qw(getCombinClass NFD); + +Suppose you wanted C<ARGUMENTS> to be an empty list, I<i.e.>, to have the +effect of: use MODULE (); -you can't do this with the C<if> pragma; however, you can achieve +You can't do this with the C<if> pragma; however, you can achieve exactly this effect, at compile time, with: BEGIN { require MODULE if CONDITION } -=head2 EXAMPLES - -The following line is taken from the testsuite for L<File::Map>: - - 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>. -On Windows it does nothing. - -The following is used to L<deprecate> core modules beyond a certain version of Perl: +=head2 C<no if> - use if $] > 5.016, 'deprecate'; +The C<no if> construct is mainly used to deactivate categories of warnings +when those categories would produce superfluous output under specified +versions of F<perl>. -This line is taken from L<Text::Soundex> 3.04, -and marks it as deprecated beyond Perl 5.16. -If you C<use Text::Soundex> in Perl 5.18, for example, -and you have used L<warnings>, -then you'll get a warning message -(the deprecate module looks to see whether the -calling module was C<use>'d from a core library directory, -and if so, generates a warning), -unless you've installed a more recent version of L<Text::Soundex> from CPAN. +For example, the C<redundant> category of warnings was introduced in +Perl-5.22. This warning flags certain instances of superfluous arguments to +C<printf> and C<sprintf>. But if your code was running warnings-free on +earlier versions of F<perl> and you don't care about C<redundant> warnings in +more recent versions, you can call: -You can also specify to NOT use something: + use warnings; + no if $] >= 5.022, q|warnings|, qw(redundant); - no if $] ge 5.021_006, warnings => "locale"; + my $test = { fmt => "%s", args => [ qw( x y ) ] }; + my $result = sprintf $test->{fmt}, @{$test->{args}}; -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 -generate an unknown warning category error. +The C<no if> construct assumes that a module or pragma has correctly +implemented an C<unimport()> method -- but most modules and pragmata have not. +That explains why the C<no if> construct is of limited applicability. =head1 BUGS -The current implementation does not allow specification of the -required version of the module. +The current implementation does not allow specification of the required +version of the module. =head1 SEE ALSO diff --git a/dist/if/t/if.t b/dist/if/t/if.t index 4a2b351..827d93c 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 => 18; -my $v_plus = $] + 1; +my $v_plus = $] + 1; my $v_minus = $] - 1; unless (eval 'use open ":std"; 1') { @@ -12,29 +12,85 @@ 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"; + + { + SKIP: { + unless ($] >= 5.018) { + skip "bigrat apparently not testable prior to perl-5.18", 4; + } + note(q|strict "subs" : 'use if' : condition false|); + eval "use if (0 > 1), q|bigrat|, qw(hex oct);"; + ok (! main->can('hex'), "Cannot call bigrat::hex() in importing package"); + ok (! main->can('oct'), "Cannot call bigrat::oct() in importing package"); + + note(q|strict "subs" : 'use if' : condition true|); + eval "use if (1 > 0), q|bigrat|, qw(hex oct);"; + ok ( main->can('hex'), "Can call bigrat::hex() in importing package"); + ok ( main->can('oct'), "Can call bigrat::oct() in importing package"); + } + } + + { + note(q|strict "subs" : 'no if' : condition variable|); + note(($] >= 5.022) ? "Recent enough Perl: $]" : "Older Perl: $]"); + use warnings; + SKIP: { + unless ($] >= 5.022) { + skip "Redundant argument warning not available in pre-5.22 perls", 4; + } + + { + no if $] >= 5.022, q|warnings|, qw(redundant); + my ($test, $result, $warn); + local $SIG{__WARN__} = sub { $warn = shift }; + $test = { fmt => "%s", args => [ qw( x y ) ] }; + $result = sprintf $test->{fmt}, @{$test->{args}}; + is($result, $test->{args}->[0], "Got expected string"); + ok(! $warn, "Redundant argument warning suppressed"); + } + + { + use if $] >= 5.022, q|warnings|, qw(redundant); + my ($test, $result, $warn); + local $SIG{__WARN__} = sub { $warn = shift }; + $test = { fmt => "%s", args => [ qw( x y ) ] }; + $result = sprintf $test->{fmt}, @{$test->{args}}; + is($result, $test->{args}->[0], "Got expected string"); + like($warn, qr/Redundant argument in sprintf/, + "Redundant argument warning generated and capture"); + } + } + } +} -- 2.7.4
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 413b
On Fri, 26 Jan 2018 02:07:55 GMT, jkeenan wrote: Show quoted text
> Thanks for the suggestion. I believe I now have better documentation > and testing of 'if'. Please review the new patch attached, 132732- > 0001-if-module-clarify-documentation-and-test-more-thorou.patch. > > Thank you very much.
I plan to apply the patch on Thu, Feb 01 unless there is objection. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
CC: <perl5-porters [...] perl.org>
Date: Tue, 30 Jan 2018 15:24:13 +1100
Subject: Re: [perl #132732] use if - behaviour does not match documentation
To: <perlbug-followup [...] perl.org>
From: <sisyphus1 [...] optusnet.com.au>
Download (untitled) / with headers
text/plain 1.3k
Show quoted text
-----Original Message----- From: James E Keenan via RT Sent: Tuesday, January 30, 2018 10:10 AM To: OtherRecipients of perl Ticket #132732: Cc: perl5-porters@perl.org Subject: [perl #132732] use if - behaviour does not match documentation .....
>> Please review the new patch attached, 132732- >> 0001-if-module-clarify-documentation-and-test-more-thorou.patch. >>
> > I plan to apply the patch on Thu, Feb 01 unless there is objection.
Jim, That all looks pretty good to me, and tests fine. The last paragraph in the "SEE ALSO" section of the POD (which pre-dates your rewrite) threw me for a few seconds. My perldoc utility renders it simply as: provide can be used to select one of several possible modules to load, based on what version of Perl is running. Probably not so confusing if you know that there's a module named "provide" - but I didn't know that, and it's not part of core. Maybe something like: =~ s/L<provide>/The L<provide> module from CPAN/ Also, the end of the sentence ("based on what version of Perl is running") makes me grimace a bit, as do the replacements that I come up with - namely, "based on the version of Perl that is running" or just "based on the version of Perl". So I'm not going to make *any* suggestions about that. Thanks for being thorough ... and for doing all the work. Cheers, Rob
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Tue, 30 Jan 2018 04:25:40 GMT, sisyphus wrote: Show quoted text
> > > -----Original Message----- > From: James E Keenan via RT > Sent: Tuesday, January 30, 2018 10:10 AM > To: OtherRecipients of perl Ticket #132732: > Cc: perl5-porters@perl.org > Subject: [perl #132732] use if - behaviour does not match > documentation > > ..... >
> >> Please review the new patch attached, 132732- > >> 0001-if-module-clarify-documentation-and-test-more-thorou.patch. > >>
> > > > I plan to apply the patch on Thu, Feb 01 unless there is objection.
> > Jim, > > That all looks pretty good to me, and tests fine. > > The last paragraph in the "SEE ALSO" section of the POD (which pre- > dates > your rewrite) threw me for a few seconds. My perldoc utility renders > it > simply as: > > provide can be used to select one of several possible modules to load, > based on what version of Perl is running. > > Probably not so confusing if you know that there's a module named > "provide" - but I didn't know that, and it's not part of core. > > Maybe something like: > =~ s/L<provide>/The L<provide> module from CPAN/ > > Also, the end of the sentence ("based on what version of Perl is > running") > makes me grimace a bit, as do the replacements that I come up with - > namely, > "based on the version of Perl that is running" or just "based on the > version > of Perl". > So I'm not going to make *any* suggestions about that. > > Thanks for being thorough ... and for doing all the work. > > Cheers, > Rob
Pushed to blead in 1654584e05038fee2cc4307f292f18e445d0e50f with some documentation touch-ups as suggested by Rob. Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Download (untitled) / with headers
text/plain 317b
Thank you for filing this report. You have helped make Perl better. With the release yesterday of Perl 5.28.0, this and 185 other issues have been resolved. Perl 5.28.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.28.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org