Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special treatment of $a and $b with 'use strict' - tighten up a bit #15797

Open
p5pRT opened this issue Jan 6, 2017 · 23 comments
Open

Special treatment of $a and $b with 'use strict' - tighten up a bit #15797

p5pRT opened this issue Jan 6, 2017 · 23 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Jan 6, 2017

Migrated from rt.perl.org#130523 (status was 'open')

Searchable as RT130523$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 6, 2017

From @epa

Created by @epa

I understand that the variables $a and $b are handled specially
because they can be used in a 'sort' block. So even under 'use strict'
they will be treated as global variables if not declared with 'my'.

However, if the program does have an explicit 'my $a' or 'my $b',
I suggest the handling can be tightened a little bit. For example

  if ($foo) {
  my $a = 5;
  bar($a);
  }
  else {
  baz($a);
  }

In the 'else' branch the global $a is being used. But only a moment
ago the programmer declared $a to be a lexically scoped variable, in
another scope in the same file.

Is it possible that if 'my $a' is seen in a file, later uses of
undeclared '$a' in that file, which aren't directly inside a sort {}
block, can give a warning?

  Global symbol "$a" seen, although "my $a" has been seen elsewhere in this file

I think this is about the weakest diagnostic possible to improve the
situation, since anything stronger (like requiring $a and $b to be
declared as normal under 'use strict', or treating them differently if
'my $a' has been seen anywhere in the program) would break too much
existing code, or produce too many false positives.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.22.2:

Configured by Red Hat, Inc. at Fri Nov  4 14:35:02 UTC 2016.

Summary of my perl5 (revision 5 version 22 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=4.7.9-200.fc24.x86_64, archname=x86_64-linux-thread-multi
    uname='linux buildvm-12.phx2.fedoraproject.org 4.7.9-200.fc24.x86_64 #1 smp thu oct 20 14:26:16 utc 2016 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=none -Dccflags=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -Dldflags=-Wl,-z,relro  -Dccdlflags=-Wl,--enable-new-dtags -Wl,-z,relro  -Dlddlflags=-shared -Wl,-z,relro  -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.22.2 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -fwrapv -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='  -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -fwrapv -fno-strict-aliasing -I/usr/local/include'
    ccversion='', gccversion='5.3.1 20160406 (Red Hat 5.3.1-6)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags ='-Wl,-z,relro  -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib64 /lib64 /usr/lib64 /usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib
    libs=-lpthread -lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lresolv -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.22.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.22'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags -Wl,-z,relro '
    cccdlflags='-fPIC', lddlflags='-shared -Wl,-z,relro  -L/usr/local/lib -fstack-protector-strong'

Locally applied patches:
    Fedora Patch1: Removes date check, Fedora/RHEL specific
    Fedora Patch3: support for libdir64
    Fedora Patch4: use libresolv instead of libbind
    Fedora Patch5: USE_MM_LD_RUN_PATH
    Fedora Patch6: Skip hostname tests, due to builders not being network capable
    Fedora Patch7: Dont run one io test due to random builder failures
    Fedora Patch15: Define SONAME for libperl.so
    Fedora Patch16: Install libperl.so to -Dshrpdir value
    Fedora Patch22: Document Math::BigInt::CalcEmu requires Math::BigInt (CPAN RT#85015)
    Fedora Patch26: Make *DBM_File desctructors thread-safe (RT#61912)
    Fedora Patch27: Make PadlistNAMES() lvalue again (CPAN RT#101063)
    Fedora Patch28: Make magic vtable writable as a work-around for Coro (CPAN RT#101063)
    Fedora Patch29: Fix duplicating PerlIO::encoding when spawning threads (RT#31923)
    Fedora Patch30: Do not let XSLoader load relative paths (CVE-2016-6185)
    Fedora Patch31: Avoid loading optional modules from default . (CVE-2016-1238)
    Fedora Patch32: Fix a crash in lexical scope warnings (RT#128597)
    Fedora Patch33: Do not mangle errno from failed socket calls (RT#128316)
    Fedora Patch34: Fix crash in "evalbytes S" (RT#129196)
    Fedora Patch35: Fix crash in "evalbytes S" (RT#129196)
    Fedora Patch36: Fix crash in "evalbytes S" (RT#129196)
    Fedora Patch37: Fix crash in splice (RT#129164, RT#129166, RT#129167)
    Fedora Patch38: Fix string overrun in Perl_gv_fetchmethod_pvn_flags (RT#129267)
    Fedora Patch39: Fix string overrun in Perl_gv_fetchmethod_pvn_flags (RT#129267)
    Fedora Patch40: Fix string overrun in Perl_gv_fetchmethod_pvn_flags (RT#129267)
    Fedora Patch41: Fix string overrun in Perl_gv_fetchmethod_pvn_flags (RT#129267)
    Fedora Patch42: Fix string overrun in Perl_gv_fetchmethod_pvn_flags (RT#129267)
    Fedora Patch43: Fix crash when matching UTF-8 string with non-UTF-8 substrings (RT#129350)
    Fedora Patch44: Fix parsing perl options in shell bang line (RT#129336)
    Fedora Patch45: Fix firstchar bitmap under UTF-8 with prefix optimization (RT#129950)
    Fedora Patch46: Avoid infinite loop in h2xs tool if enum and type have the same name (RT130001)
    Fedora Patch47: Fix stack handling when calling chdir without an argument (RT#129130)
    Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux
    Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux


@INC for perl 5.22.2:
    /home/eda/lib64/perl5/
    /usr/local/lib64/perl5
    /usr/local/share/perl5
    /usr/lib64/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib64/perl5
    /usr/share/perl5


Environment for perl 5.22.2:
    HOME=/home/eda
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LC_CTYPE=en_GB.UTF-8
    LC_MESSAGES=en_GB.UTF-8
    LC_MONETARY=en_GB.UTF-8
    LC_NUMERIC=en_GB.UTF-8
    LC_TIME=en_GB.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin
    PERL5LIB=/home/eda/lib64/perl5/
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 6, 2017

From @Abigail

On Fri, Jan 06, 2017 at 09​:09​:54AM -0800, Ed Avis wrote​:

# New Ticket Created by "Ed Avis"
# Please include the string​: [perl #130523]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130523 >

This is a bug report for perl from eda@​waniasset.com,
generated with the help of perlbug 1.40 running under perl 5.22.2.

-----------------------------------------------------------------
[Please describe your issue here]

I understand that the variables $a and $b are handled specially
because they can be used in a 'sort' block. So even under 'use strict'
they will be treated as global variables if not declared with 'my'.

However, if the program does have an explicit 'my $a' or 'my $b',
I suggest the handling can be tightened a little bit. For example

if \($foo\) \{
    my $a = 5;
    bar\($a\);
\}
else \{
    baz\($a\);
\}

In the 'else' branch the global $a is being used. But only a moment
ago the programmer declared $a to be a lexically scoped variable, in
another scope in the same file.

Is it possible that if 'my $a' is seen in a file, later uses of
undeclared '$a' in that file, which aren't directly inside a sort {}
block, can give a warning?

I don't think it's possible to determine this at compile time, other
than maybe a handful of cases. Given Perl's dynamic nature, it smells
too much like solving the halting problem.

What if the fragment above is as follows​:

  sub something {
  my $foo = 0;
  if ($foo) {
  my $a = 5;
  bar($a);
  }
  else {
  baz($a);
  }
  ...;
  }

  sub something_else {
  ...;
  }

  my $func = @​ARGV ? \&something : \&something_else;

  sort $func 1, 2, 3;

Now something() is called from a sort block, but you don't know
that at compile time.

Abigail

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 6, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 7, 2017

From @epa

To clarify, by 'directly inside a sort {} block' I meant lexically inside it, which is known at compile time.

The conditions I suggested for triggering the warning (only when 'my $a' is in the same file, undeclared '$a' is seen, and the code isn't directly inside a sort {} block) are an attempt to improve the current handling of $a and $b under 'use strict' -- which *is* rather a mess -- while trying hard to avoid any false positives. While it is possible to contrive code examples which trigger the warning and yet work as intended, that is the case for any warning.

In an ideal world I would like to have some stronger change, such as removing the special treatment for $a and $b altogether under 'use 5.30' or later, unless directly inside a sort {} block. But that would be too big a compatibility break. Hence the suggestion for a warning, which while not perfect, would be an improvement.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 8, 2017

From @Abigail

On Sat, Jan 07, 2017 at 12​:27​:24PM -0800, Ed Avis via RT wrote​:

To clarify, by 'directly inside a sort {} block' I meant lexically inside it, which is known at compile time.

The conditions I suggested for triggering the warning (only when 'my $a' is in the same file, undeclared '$a' is seen, and the code isn't directly inside a sort {} block) are an attempt to improve the current handling of $a and $b under 'use strict' -- which *is* rather a mess -- while trying hard to avoid any false positives. While it is possible to contrive code examples which trigger the warning and yet work as intended, that is the case for any warning.

But doesn't that mean that my example would now trigger a warning?
Which would mean it triggers a warning where $a/$b *are* being used
in a sorting operation.

In an ideal world I would like to have some stronger change, such as removing the special treatment for $a and $b altogether under 'use 5.30' or later, unless directly inside a sort {} block. But that would be too big a compatibility break. Hence the suggestion for a warning, which while not perfect, would be an improvement.

Abigail

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 8, 2017

From dcmertens.perl@gmail.com

On Sun, Jan 8, 2017 at 6​:38 AM, Abigail <abigail@​abigail.be> wrote​:

On Sat, Jan 07, 2017 at 12​:27​:24PM -0800, Ed Avis via RT wrote​:

To clarify, by 'directly inside a sort {} block' I meant lexically
inside it, which is known at compile time.

The conditions I suggested for triggering the warning (only when 'my $a'
is in the same file, undeclared '$a' is seen, and the code isn't directly
inside a sort {} block) are an attempt to improve the current handling of
$a and $b under 'use strict' -- which *is* rather a mess -- while trying
hard to avoid any false positives. While it is possible to contrive code
examples which trigger the warning and yet work as intended, that is the
case for any warning.

But doesn't that mean that my example would now trigger a warning?
Which would mean it triggers a warning where $a/$b *are* being used
in a sorting operation.

I think that was Ed's very point. Chances are pretty good that any
maintainer reading Abigail's code will be unsure of the original author's
intent. If the original programmer had a warning, they would probably take
it as a sign to use a different variable name for the lexical variable​: a
more descriptive name should probably be found. If you don't want to find
that descriptive name, if you really want tot use "my $a", then don't
enable that warning.

David

--
"Debugging is twice as hard as writing the code in the first place.
  Therefore, if you write the code as cleverly as possible, you are,
  by definition, not smart enough to debug it." -- Brian Kernighan

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 9, 2017

From @iabyn

On Sun, Jan 08, 2017 at 07​:10​:23AM -0500, David Mertens wrote​:

I think that was Ed's very point. Chances are pretty good that any
maintainer reading Abigail's code will be unsure of the original author's
intent. If the original programmer had a warning, they would probably take
it as a sign to use a different variable name for the lexical variable​: a
more descriptive name should probably be found. If you don't want to find
that descriptive name, if you really want tot use "my $a", then don't
enable that warning.

Should the following src file emit a warning​:

  sub foo {
  my ($a, $b, $c) = @​_;
  ...
  }

  # ... 1000 lines of code ...

  sub sort_by_foo { $foo[$a] <=> $foo[$b] }
  my @​sorted = sort \&sort_by_foo, @​data;

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
  -- Things That Never Happen in "Star Trek" #10

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 9, 2017

From @epa

Dave M, yes that is an example of a false positive. The code relies on the global $a and $b being set when the sort subroutine is called.

My personal preference is that the code should give the warning, and the programmer could instead write 'our $a, $b' to express the intention to pick up the globals; or disable the warning; or disable 'use strict "vars"'; or pick other names than $a and $b in the earlier code; or change sort_by_foo to pick up its arguments in @​_ and give it the ($$) prototype.

I will note in passing that the variable names $a and $b are quite commonly used in example code in the perl documentation, as 'ordinary' variable names and not related to sorting.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 9, 2017

From @iabyn

On Mon, Jan 09, 2017 at 04​:46​:53AM -0800, Ed Avis via RT wrote​:

Dave M, yes that is an example of a false positive. The code relies on
the global $a and $b being set when the sort subroutine is called.

My personal preference is that the code should give the warning

I would be opposed. I think my example was representative of many source
files, and thus we could expect many, many working scripts to suddenly
start emitting warnings.

http​://grep.cpan.me/ shows​:

  * more than 1000 distributions with /my\s*\$a\b/
  * more than 1000 distributions with /my\s*\(\$a\b/

so using $a as a lexical var is very common, and using its presence as
as moderator for making package $a no longer warnings-exempt isn't going
to moderate all that much.

--
A major Starfleet emergency breaks out near the Enterprise, but
fortunately some other ships in the area are able to deal with it to
everyone's satisfaction.
  -- Things That Never Happen in "Star Trek" #13

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 9, 2017

From @epa

The example you gave may be representative - certainly code like that, which uses a named sort comparator and uses global $a,$b, is not unknown. But the proposed warning would not affect all 1000+ distributions that use $a as a lexical variable.

Only the case where $a is used as a lexical variable, and also as an undeclared global variable in the same file (outside a sort subroutine) would be affected. That is surely fewer than 1000 though it will likely be nonzero.

I agree that lexical $a is common. The motivation for this bug report is to make it work better and be less surprising should the 'my $a' be forgotten. You are probably right that "using its presence as a moderator... isn't going to moderate all that much". Perhaps it cuts the false positives by 50%, which is not great.

I wonder whether a whole-file check could look for both $a and $b? If both of them are used without declaration, it's surely deliberate. If just $a by itself pops up in one place, and the code has 'use strict', then it does look odd. (Nobody writes a sort comparator that looks at just one of the two values.) But this would require deferring the warning until end of file, which I don't know if it's possible.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @xsawyerx

On 01/09/2017 05​:25 PM, Ed Avis via RT wrote​:

The example you gave may be representative - certainly code like that, which uses a named sort comparator and uses global $a,$b, is not unknown. But the proposed warning would not affect all 1000+ distributions that use $a as a lexical variable.

Only the case where $a is used as a lexical variable, and also as an undeclared global variable in the same file (outside a sort subroutine) would be affected. That is surely fewer than 1000 though it will likely be nonzero.

Honestly, this feels to me like a Perl​::Critic policy, rather than a new
core warning. Any new warning that might start triggering on
unsuspecting code. Additionally, the fact that it's hard to follow (even
for those involved in the thread, it seems) when exactly this would
trigger a warning, makes it a hard-to-explain warning which might need
to be refined over and over, as we suddenly come up with another false
positive or situation of which we haven't thought.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @epa

I agree, it's not pretty. I have contorted it to try to squeeze it into the least intrusive shape while still making some improvement to the current handling of $a and $b.

(Do others agree with me that the treatment of $a and $b under 'use strict' is a mess? If the consensus is that there's no problem, there is no need for anything to change. Although I would suggest in that case picking different example variable names in the perl documentation.)

Perhaps instead of some long chain of heuristic tests, there should be a simpler but more drastic change which can be opt-in. How about a pragma which just stops the special treatment of $a and $b altogether outside a sort {} block? So if you still want to use them in sort subroutines you declare them with 'our ($a, $b)'.

  use strict 'vars_ab';
  $a += 5; # Global symbol "$a" requires explicit package name
  sort { $a <=> $b } (); # still allowed

As this is opt-in, the issue of false positives wouldn't arise.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @xsawyerx

On 01/10/2017 12​:33 PM, Ed Avis via RT wrote​:

I agree, it's not pretty. I have contorted it to try to squeeze it into the least intrusive shape while still making some improvement to the current handling of $a and $b.

(Do others agree with me that the treatment of $a and $b under 'use strict' is a mess? If the consensus is that there's no problem, there is no need for anything to change. Although I would suggest in that case picking different example variable names in the perl documentation.)

I try not to form an opinion solely on whether people agree with you or
not. If you're right, you're right. :)

In this case, I think it would be nice to tighten up $a and $b, but it's
hard to pin down when exactly and while the definition seems crude, the
false positives might seem high, and perhaps core is not the place for it.

Perhaps instead of some long chain of heuristic tests, there should be a simpler but more drastic change which can be opt-in.

That's how I see any Perl​::Critic policy for it.

How about a pragma which just stops the special treatment of $a and $b altogether outside a sort {} block? So if you still want to use them in sort subroutines you declare them with 'our ($a, $b)'.

use strict 'vars\_ab';
$a \+= 5;               \# Global symbol "$a" requires explicit package name
sort \{ $a \<=> $b \} \(\); \# still allowed

As this is opt-in, the issue of false positives wouldn't arise.

I don't think "strict.pm" would be the best point. You can introduce a
separate pragma, such as "strict_ab_vars" or "abvars" ("no abvars") or a
must better name, that allows you to install it separate from perl and
run wherever you want instead of waiting for another version to come out.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @Grinnz

On Tue, Jan 10, 2017 at 7​:34 AM, Sawyer X <xsawyerx@​gmail.com> wrote​:

On 01/10/2017 12​:33 PM, Ed Avis via RT wrote​:

I agree, it's not pretty. I have contorted it to try to squeeze it into
the least intrusive shape while still making some improvement to the
current handling of $a and $b.

(Do others agree with me that the treatment of $a and $b under 'use
strict' is a mess? If the consensus is that there's no problem, there is
no need for anything to change. Although I would suggest in that case
picking different example variable names in the perl documentation.)

This is definitely a good idea if someone is up for it. I usually just
switch $a and $b for $x and $y.

Perhaps instead of some long chain of heuristic tests, there should be a
simpler but more drastic change which can be opt-in.

That's how I see any Perl​::Critic policy for it.

I don't know if there is a core Perl​::Critic policy for it, but my policy
bundle has one​:

https://metacpan.org/pod/Perl::Critic::Policy::Freenode::DollarAB

-Grinnz

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @epa

Thanks for the pointers to perlcritic policies. The idea of the bug report is to 'fix the bug' with use strict 'vars', which works excellently but has an annoying blind spot when it comes to $a and $b. Just as use strict 'vars' is too useful and too important to be relegated to a perlcritic policy, I suggest that fixing it for $a and $b is also something that should work out of the box, should the programmer choose it.

(I don't know about everyone else, but when hacking on code I do not run perlcritic on it during each edit-run cycle. I rely on Perl's compile-time checking of variable names and other things to give quick feedback of typos or missed declarations. Hence the wish to turn off the special case which doesn't report on undeclared $a and $b.)

Since use strict 'vars' is part of the perl core, I think that its variant 'vars_ab' (which is really just "what it should have done all along" in some sense) belongs in core too. This is not to say that anybody must spend their time working on it, but the existence of a separate critic policy is not a good reason to exclude the check from normal compile-time checking. After all, this is not something exotic or weird being proposed, rather it is the current behaviour on $a and $b which is a bit odd and this proposal is to regularize it.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From zefram@fysh.org

Ed Avis via RT wrote​:

Since use strict 'vars' is part of the perl core, I think that its
variant 'vars_ab' (which is really just "what it should have done all
along" in some sense)

Shouldn't "what it should have done all along" include $STDIN and the
rest too?

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @rjbs

* Ed Avis via RT <perlbug-followup@​perl.org> [2017-01-10T06​:33​:12]

[ stuff about $a and $b ]

I don't like your proposed solutions to the problem of $a and $b. I should be
able to use $a and $b in named subroutines that will be used by sort.

Your "our" solution says "I do plan to use $a and $b," but says nothing about
being certain they're for sorting.

If I was going to suggest any change -- and I don't think I am -- then I'd say
that $a and $b should, in a state of nature, produce warnings if read from.

  use warnings;
  sub xorly { $b xor $a }
  my $x = sort xorly @​array; # no warning
  my $y = xorly(); # warning, $a and $b used outside of sort

I agree that $a and $b's weirdness can lead to bugs. I'm not sure there is a
good action to take that is proportional to the problem.

--
rjbs

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 10, 2017

From @epa

Zefram asked​:

include $STDIN and the rest too?

You may be right. To me $STDIN looks more obviously magic. The perl documentation is not full of example code which uses $STDIN as an ordinary variable name.

As far as I know $a and $b are the only builtin variables which don't begin with uppercase or punctuation. And they are probably the only ones you would pick unknowingly to use in your program.

(It might be a good idea to have a warning on 'my $STDIN', noting that there is a builtin global variable of the same name and you probably want to rename your lexical variable to something else to avoid craziness. But I think that is not the right answer for $a and $b.)

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 11, 2017

From @Abigail

On Tue, Jan 10, 2017 at 03​:33​:12AM -0800, Ed Avis via RT wrote​:

I agree, it's not pretty. I have contorted it to try to squeeze it
into the least intrusive shape while still making some improvement to
the current handling of $a and $b.

(Do others agree with me that the treatment of $a and $b under 'use
strict' is a mess? If the consensus is that there's no problem, there
is no need for anything to change. Although I would suggest in that
case picking different example variable names in the perl documentation.)

A mess? Not really. $a and $b are treated special, and so are a dozen
other variables. Piling more special handling cases for them are leaps
in the wrong direction.

With hindsight, sort would have never used $a, and $b. But if we'd would
have a time machine, and retroactively removed every oddity from Perl,
we'd all be programming in a different language.

Perhaps instead of some long chain of heuristic tests, there should
be a simpler but more drastic change which can be opt-in. How about a
pragma which just stops the special treatment of $a and $b altogether
outside a sort {} block? So if you still want to use them in sort
subroutines you declare them with 'our ($a, $b)'.

use strict 'vars\_ab';
$a \+= 5;               \# Global symbol "$a" requires explicit package name
sort \{ $a \<=> $b \} \(\); \# still allowed

As this is opt-in, the issue of false positives wouldn't arise.

If people write pragmas which enforce whatever they think $a and $b
should behave, and put them on CPAN, that's fine. I just don't think
this should be the default behaviour.

How big is the problem you're trying to solve anyway? How wide spread is
the use of $a and $b in real code, and in how many of those cases is
there a problem because the variables are package variables, and are
used outside of a sort? (Just the mere fact a variable is a package
variable doesn't mean there's something wrong with the code).

Abigail

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 13, 2017

From @epa

I think the use of $a and $b is fairly widespread, perhaps a bit less common than $i and $j or $x and $y. They are natural choices to reach for when you want to pick two arbitrary variables for a loop iterator or whatever. That is just my opinion, but it is backed up by a quick glance through perlfunc. Those code examples were written by experienced Perl developers, who knew about the special sort variables, and they still picked $a or $b a lot of the time as a 'plain vanilla' variable.

I prefer to put it this way. 'use strict' is an excellent feature of perl and a big timesaver in the edit-test cycle. Currently $a and $b don't get the benefit of it, which is a pity. They are excluded as a special case. Some way to turn that special case off for new development would regularize the language a little and avoid some 'WTF moments' when first writing and testing code.

It is always true that you can write whatever pragma you want and put it on CPAN, but here it's not some wild and wacky feature that is wanted, just to apply the same boring 'use strict' rules that apply to every other lowercase variable name.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 14, 2017

From @xsawyerx

On 01/13/2017 11​:31 AM, Ed Avis via RT wrote​:

I think the use of $a and $b is fairly widespread, perhaps a bit less common than $i and $j or $x and $y. They are natural choices to reach for when you want to pick two arbitrary variables for a loop iterator or whatever. That is just my opinion, but it is backed up by a quick glance through perlfunc. Those code examples were written by experienced Perl developers, who knew about the special sort variables, and they still picked $a or $b a lot of the time as a 'plain vanilla' variable.

I prefer to put it this way. 'use strict' is an excellent feature of perl and a big timesaver in the edit-test cycle. Currently $a and $b don't get the benefit of it, which is a pity. They are excluded as a special case. Some way to turn that special case off for new development would regularize the language a little and avoid some 'WTF moments' when first writing and testing code.

While I understanding the case you're making, I still believe this isn't
a good direction. The combination of this being in core only (available
only in, say, 5.28 and above), adding something to strict.pm which it
never covered (suddenly causing compile-time errors or run-time
warnings), and it being an illusive situation to pin down and explain,
this probably is enough reason to avoid it. There are alternative to
that do not add confusion in explaining, in figuring out, in using prior
to whatever new version, and not being core-only, such as the Perl
Critic policy mentioned in this thread.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 16, 2017

From @epa

Thanks for your feedback. Is there some other way that this issue could be addressed?

For myself, I would be quite happy to say 'no sort "ab"' and have the sort builtin no longer set the $a and $b global variables but always pass them onto the stack instead. Then these two variable names would become entirely ordinary.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jan 17, 2017

From @xsawyerx

On 01/16/2017 02​:54 PM, Ed Avis via RT wrote​:

Thanks for your feedback. Is there some other way that this issue could be addressed?

My position is not to introduce a change to the effect you requested.
Instead, I suggest looking for a different solution, such as the one
previous suggested.

If someone else wants to take a stab at solving this, perhaps further
discussion is valuable. Otherwise, with no one agreeing on this or
interested in working on code to address it, I can't see how this continues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant