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

disallow literal control character variables #13144

Closed
p5pRT opened this issue Aug 2, 2013 · 25 comments
Closed

disallow literal control character variables #13144

p5pRT opened this issue Aug 2, 2013 · 25 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Aug 2, 2013

Migrated from rt.perl.org#119123 (status was 'resolved')

Searchable as RT119123$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 2, 2013

From @rjbs

There are a bunch of variables in the form $^A, where A is some upper case
ASCII letter. These variables magically get stored under the name \cA. So,
these two lines of perl do the same thing​:

  say $^T;
  say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

  say $�; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was allowed. It came
up as weird / problematic here​:

  use feature 'say';
  say "version $^V";
  $^K = 123;
  say "^K​: " . $^K;
  say "cK​: " . ${"\cK"};
  say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

  version v5.12.4
  ^K​: 123
  cK​: 123
  +K​: 123

...or...

  version v5.17.10
  ^K​: 123
  cK​: 123
  +K​:

...because, more or less, \cK is whitespace now. (This might not be exactly
right, but let's roll with that.) So, we say "you can't use the literal
control character if it would be whitespace." But on EBCDIC platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to call the literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P character.

This post is a duplicate of the initial non-perlbug suggestion​:

  http​://markmail.org/message/7zvlmra2y4r3d2hv

--
rjbs

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 2, 2013

From @Tux

On Thu, 01 Aug 2013 19​:53​:30 -0700, Ricardo SIGNES (via RT)
<perlbug-followup@​perl.org> wrote​:

Rather than grump and blame EBCDIC, I think the solution is to call the literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P character.

Fully support that deprecation. +1 from me

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 2, 2013

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 2, 2013

From @khwilliamson

On 08/02/2013 05​:19 AM, H.Merijn Brand wrote​:

On Thu, 01 Aug 2013 19​:53​:30 -0700, Ricardo SIGNES (via RT)
<perlbug-followup@​perl.org> wrote​:

Rather than grump and blame EBCDIC, I think the solution is to call the literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P character.

Fully support that deprecation. +1 from me

+1

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 2, 2013

From @mauke

On 02.08.2013 04​:53, Ricardo SIGNES (via RT) wrote​:

Rather than grump and blame EBCDIC, I think the solution is to call the literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P character.

This will break one of my programs. I say go for it :-)

--
Lukas Mai <plokinom@​gmail.com>

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 5, 2013

From @demerphq

+1.

On 2 August 2013 04​:53, Ricardo SIGNES <perlbug-followup@​perl.org> wrote​:

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

There are a bunch of variables in the form $^A, where A is some upper case
ASCII letter. These variables magically get stored under the name \cA. So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was allowed. It came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might not be exactly
right, but let's roll with that.) So, we say "you can't use the literal
control character if it would be whitespace." But on EBCDIC platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to call the literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

--
rjbs

-----BEGIN PGP SIGNATURE-----
Version​: GnuPG v1.4.12 (GNU/Linux)

iQEcBAEBAgAGBQJR+x8LAAoJEOYby6cMccU5v/0IAIt1/ucIjXOrAX/O3Bu2MFQg
L7j07LlVim/OoebI14vXC26EvE8djN74LSN7cvPQIa//N/cxDgvXBt6wutoaOtAS
uy0k7ItZmJIgguTZ4jdCjdRAvWhjwJL09S0zOI3bnPBdJdvNmMsWXxVlq9Voa6k5
j0M+rD1564Ug0PTAw4IzQ9p65PksJGp6m1prSZLaGuhZ6QPmhGIRJFfo0Laapjyp
ueA8uhhSw/OFEGvSTmipl/LisYDmYRiY+dyo57d2xJAE+PfJza4TGQUN4s9kAeHJ
7TGKjmuYmDQGkMhsDZXEbZDARhFJcHWfmxA8mBANzSpzO1hJ122glznWeKhW9Do=
=ba4z
-----END PGP SIGNATURE-----

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @Hugmeir

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some upper case
ASCII letter. These variables magically get stored under the name \cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was allowed. It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might not be
exactly
right, but let's roll with that.) So, we say "you can't use the literal
control character if it would be whitespace." But on EBCDIC platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to call the
literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in behavior,
but fixes a bug in how literal control variables are parsed. That bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

Also, this required some changes to t/base/lex.t, which should make future
changes to that file in less horrid.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @cpansprout

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some
upper case
ASCII letter. These variables magically get stored under the name \cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was allowed. It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might not be
exactly
right, but let's roll with that.) So, we say "you can't use the literal
control character if it would be whitespace." But on EBCDIC
platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to call the
literal
control form bogus, value-free, and best disposed of. I propose that we
deprecate this form of literal variable in 5.19 for removal in 5.21. If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

Could you put that last paragraph in the commit message, too? (And, in
future, anything that could use clarification.)

Does the newline-skipping work outside of string eval? :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @cpansprout

On Sat Aug 31 10​:32​:20 2013, sprout wrote​:

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some
upper case
ASCII letter. These variables magically get stored under the name
\cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was
allowed. It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might not be
exactly
right, but let's roll with that.) So, we say "you can't use the
literal
control character if it would be whitespace." But on EBCDIC
platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to
call the
literal
control form bogus, value-free, and best disposed of. I propose
that we
deprecate this form of literal variable in 5.19 for removal in
5.21. If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That
bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

Could you put that last paragraph in the commit message, too? (And, in
future, anything that could use clarification.)

Does the newline-skipping work outside of string eval? :-)

Even if it doesn’t don’t worry about it, as I am in the middle of fixing
bugs of that sort, and our work would probably conflict.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @Hugmeir

On Sat, Aug 31, 2013 at 2​:32 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some
upper case
ASCII letter. These variables magically get stored under the name \cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was allowed.
It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might not be
exactly
right, but let's roll with that.) So, we say "you can't use the
literal
control character if it would be whitespace." But on EBCDIC
platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to call the
literal
control form bogus, value-free, and best disposed of. I propose that
we
deprecate this form of literal variable in 5.19 for removal in 5.21.
If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

Could you put that last paragraph in the commit message, too? (And, in
future, anything that could use clarification.)

Sure, done.

Does the newline-skipping work outside of string eval? :-)

Bugger, it doesn't do it properly -- ${\nfoo} doesn't warn about the \n
being deprecated because it's correctly being skipped over, but the bug
that I mentioned is still present outside of a string eval. Nice catch!

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @cpansprout

On Sat Aug 31 11​:16​:34 2013, Hugmeir wrote​:

On Sat, Aug 31, 2013 at 2​:32 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some
upper case
ASCII letter. These variables magically get stored under the
name \cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was
allowed.
It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might
not be
exactly
right, but let's roll with that.) So, we say "you can't use the
literal
control character if it would be whitespace." But on EBCDIC
platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to
call the
literal
control form bogus, value-free, and best disposed of. I propose
that
we
deprecate this form of literal variable in 5.19 for removal in
5.21.
If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a
literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace
for
non-digit characters, by also skipping over \n -- previously it
only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That
bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}";
CORE​::say $@​'

Could you put that last paragraph in the commit message, too? (And,
in
future, anything that could use clarification.)

Sure, done.

Does the newline-skipping work outside of string eval? :-)

Bugger, it doesn't do it properly -- ${\nfoo} doesn't warn about the
\n
being deprecated because it's correctly being skipped over, but the
bug
that I mentioned is still present outside of a string eval. Nice
catch!

And I’ve just discovered something else, too​:

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print
Dumper "".eval "*{\nOIN}"'
$VAR1 = "*main​::\nOIN";

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print
Dumper "".*{' -e 'OIN}'
$VAR1 = "*main​::OIN";

Does your patch address that? Basically I’m writing ${^JOIN} (no such
variable, I know) with a literal ^J aka \n. I thought that disallowing
literal control characters here was pedantic. But now I fully support it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Aug 31, 2013

From @Hugmeir

On Sat, Aug 31, 2013 at 3​:37 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Aug 31 11​:16​:34 2013, Hugmeir wrote​:

On Sat, Aug 31, 2013 at 2​:32 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

On Thu, Aug 1, 2013 at 11​:53 PM, Ricardo SIGNES
<perlbug-followup@​perl.org>wrote​:

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

There are a bunch of variables in the form $^A, where A is some
upper case
ASCII letter. These variables magically get stored under the
name \cA.
So,
these two lines of perl do the same thing​:

say $^T;
say ${"\cT"};

What's more, it's legal to put a literal ^T in the perl program​:

say $ ; # <-- that's a literal ^T after the dollar sign there

Why? So far, I have yet to find a great reason that it was
allowed.
It
came
up as weird / problematic here​:

use feature 'say';
say "version $^V";
$^K = 123;
say "^K​: " . $^K;
say "cK​: " . ${"\cK"};
say "+K​: " . $^K; # <-- literal ^K in my source

This program prints​:

version v5.12.4
^K​: 123
cK​: 123
+K​: 123

...or...

version v5.17.10
^K​: 123
cK​: 123
+K​:

...because, more or less, \cK is whitespace now. (This might
not be
exactly
right, but let's roll with that.) So, we say "you can't use the
literal
control character if it would be whitespace." But on EBCDIC
platforms \cT
would be whitespace!

Rather than grump and blame EBCDIC, I think the solution is to
call the
literal
control form bogus, value-free, and best disposed of. I propose
that
we
deprecate this form of literal variable in 5.19 for removal in
5.21.
If
you
want $^P, you say $^P or ${"\cP"} and can no longer use a
literal ^P
character.

This post is a duplicate of the initial non-perlbug suggestion​:

http​://markmail.org/message/7zvlmra2y4r3d2hv

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

It has a minor tweak to how S_scan_ident() skips over whitespace
for
non-digit characters, by also skipping over \n -- previously it
only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That
bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}";
CORE​::say $@​'

Could you put that last paragraph in the commit message, too? (And,
in
future, anything that could use clarification.)

Sure, done.

Does the newline-skipping work outside of string eval? :-)

Bugger, it doesn't do it properly -- ${\nfoo} doesn't warn about the
\n
being deprecated because it's correctly being skipped over, but the
bug
that I mentioned is still present outside of a string eval. Nice
catch!

And I’ve just discovered something else, too​:

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print
Dumper "".eval "*{\nOIN}"'
$VAR1 = "*main​::\nOIN";

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print
Dumper "".*{' -e 'OIN}'
$VAR1 = "*main​::OIN";

Does your patch address that?

Augh, that's nasty. I suspected something like that was possible but didn't
have a test case. The patch does fix it​:
$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print Dumper
"".eval "*{\nOIN}"'
$VAR1 = "*main​::OIN";

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print Dumper
"".*{' -e 'OIN}'
$VAR1 = "*main​::OIN";

$ ./perl -Ilib -le 'use Data​::Dumper; ++$Data​::Dumper​::Useqq; print Dumper
"".eval "*{^JOIN}"'
$VAR1 = "*main​::\nOIN";

Basically I’m writing ${^JOIN} (no such

variable, I know) with a literal ^J aka \n. I thought that disallowing
literal control characters here was pedantic. But now I fully support it.

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119123

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 1, 2013

From @nwc10

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

Nice work.

I don't think that it should be applied as-is, because​:

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

That's this hunk?

Inline Patch
diff --git a/toke.c b/toke.c
index 2764709..702f637 100644
--- a/toke.c
+++ b/toke.c
@@ -9340,7 +9340,7 @@ S_scan_ident(pTHX_ char *s, const char *send, char
*dest,   if \(\*s == '\{'\) \{   bracket = s;   s\+\+; \- while \(s \< send && SPACE\_OR\_TAB\(\*s\)\) \+ while \(s \< send && \( SPACE\_OR\_TAB\(\*s\) || \*s == '\\n' \)\)   s\+\+;   \}

@​@​ -9422,7 +9422,7 @​@​ S_scan_ident(pTHX_ char *s, const char *send, char
*dest,
  *d = '\0';
  }

- while (s < send && SPACE_OR_TAB(*s))
+ while (s < send && ( SPACE_OR_TAB(*s) || *s == '\n' ))
  s++;

  /* Expect to find a closing } after consuming any trailing
whitespace.

I think that that should be its own commit (with a relevant test)

Also, this required some changes to t/base/lex.t, which should make future
changes to that file in less horrid.

I think that your useful cleanup there should go in as its own commit

Right now you have a single commit that is doing 3 things

1) cleaning up t/base/lex.t (with no functional changes)
2) fixing a long standing parse error (with changes to t/base/lex.t)
3) adding a deprecation (with changes to t/base/lex.t)

It would be lot clearer to split those 3 distinct things out.
I think it would also make sense to merge the perldelta commit that
describes the last change into the commit that makes the change.

Also, right now, when I test your branch as-is, it's failing 1 test
from lib/warnings.t, I think because your "EXPECT" expects 1 more
warning than your test code actually generates.

Thanks for tackling this.

Nicholas Clark

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 1, 2013

From @Hugmeir

On Sun, Sep 1, 2013 at 4​:53 AM, Nicholas Clark via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Aug 31 09​:25​:37 2013, Hugmeir wrote​:

This branch implements the deprecation​:

https://github.com/Hugmeir/utf8mess/tree/disallow_cntrl

Nice work.

I don't think that it should be applied as-is, because​:

It has a minor tweak to how S_scan_ident() skips over whitespace for
non-digit characters, by also skipping over \n -- previously it only
skipped over spaces and tabs. This shouldn't cause any changes in
behavior,
but fixes a bug in how literal control variables are parsed. That bug can
be reproduced on blead with​: ./perl -we 'eval "\${\cT\n}"; CORE​::say $@​'

That's this hunk?

diff --git a/toke.c b/toke.c
index 2764709..702f637 100644
--- a/toke.c
+++ b/toke.c
@​@​ -9340,7 +9340,7 @​@​ S_scan_ident(pTHX_ char *s, const char *send, char
*dest,
if (*s == '{') {
bracket = s;
s++;
- while (s < send && SPACE_OR_TAB(*s))
+ while (s < send && ( SPACE_OR_TAB(*s) || *s == '\n' ))
s++;
}

@​@​ -9422,7 +9422,7 @​@​ S_scan_ident(pTHX_ char *s, const char *send, char
*dest,
*d = '\0';
}

- while (s < send && SPACE_OR_TAB(*s))
+ while (s < send && ( SPACE_OR_TAB(*s) || *s == '\n' ))
s++;

     /\* Expect to find a closing \} after consuming any trailing

whitespace.

I think that that should be its own commit (with a relevant test)

Also, this required some changes to t/base/lex.t, which should make
future
changes to that file in less horrid.

I think that your useful cleanup there should go in as its own commit

Right now you have a single commit that is doing 3 things

1) cleaning up t/base/lex.t (with no functional changes)
2) fixing a long standing parse error (with changes to t/base/lex.t)
3) adding a deprecation (with changes to t/base/lex.t)

It would be lot clearer to split those 3 distinct things out.
I think it would also make sense to merge the perldelta commit that
describes the last change into the commit that makes the change.

Thank you for your review. This makes sense, apologies for the sloppy
branch! I've force-pushed a fixed version, but...

Also, right now, when I test your branch as-is, it's failing 1 test
from lib/warnings.t, I think because your "EXPECT" expects 1 more
warning than your test code actually generates.

This is what I get for rebasing the branch and not testing properly.
I originally added a test for ${\n\cK\n}, which should've warned on the \cK
and nothing more. However, after rebasing, it clashes with the fix added
in 6a048a6 by Karl, which disallows literal whitespace in variable names,
turning the test into a syntax error.

This lead me to realize that this was broken​:

$ perl -e 'eval "\${\nfoo} = 10; warn q{should be line 2}"'
should be line 2 at (eval 1) line 1.

Worse still, this branch broke that further if there were multiple newlines
involved.
There's other stuff involving whitespace that's broken there, too, although
this comes from way back​: ${\ntime\n{foo}} is a syntax error if any of
those two newlines is present, and the warnings from this file all have the
wrong line numbers if there any newlines involved.

I have a fix for that (sorry, Father C! I hope there's no clashes. if there
are, I'll modify the branch accordingly), but it requires reworking
S_scan_ident() to use PL_bufend rather than taking in an explicit "end of
buffer" (send) argument. This is less troublesome than it sounds, since all
but one of S_scan_ident()'s callers already pass PL_bufend as send.

I've pushed that fix in the same branch, but perhaps it should go in a
different branch/ticket.

Additionally, with the above change, I think we could simplify
the VALID_LEN_ONE_IDENT macro from this​:
  || ((((U8)(d)) < 32)
\
  && (((((U8)(d)) >= 14)
\
  || (((U8)(d)) <= 8 && (d) != 0) \
  || (((U8)(d)) == 13))))
\
  || (((U8)(d)) == toCTRL('?')))
to this​:
  || ( isCNTRL_A((U8)d) && !isSPACE(d) )

But Karl should chime in for that one.

In any case, those changes render the original test pointless, so I've
removed it.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 2, 2013

From @cpansprout

On Sun Sep 01 16​:51​:23 2013, Hugmeir wrote​:

Worse still, this branch broke that further if there were multiple
newlines
involved.
There's other stuff involving whitespace that's broken there, too,
although
this comes from way back​: ${\ntime\n{foo}} is a syntax error if any of
those two newlines is present, and the warnings from this file all
have the
wrong line numbers if there any newlines involved.

I have a fix for that (sorry, Father C! I hope there's no clashes. if
there
are, I'll modify the branch accordingly)

The work I was referring to is in blead now.

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 2, 2013

From @nwc10

On Sun, Sep 01, 2013 at 08​:50​:38PM -0300, Brian Fraser wrote​:

On Sun, Sep 1, 2013 at 4​:53 AM, Nicholas Clark via RT <

1) cleaning up t/base/lex.t (with no functional changes)
2) fixing a long standing parse error (with changes to t/base/lex.t)
3) adding a deprecation (with changes to t/base/lex.t)

It would be lot clearer to split those 3 distinct things out.
I think it would also make sense to merge the perldelta commit that
describes the last change into the commit that makes the change.

Thank you for your review. This makes sense, apologies for the sloppy
branch! I've force-pushed a fixed version, but...

You have this​:

Inline Patch
diff --git a/toke.c b/toke.c
index e125181..228fd12 100644
--- a/toke.c
+++ b/toke.c
@@ -3943,7 +3943,10 @@ S_intuit_more(pTHX_ char *s)
                weight -= seen[un_char] * 10;
                if (isWORDCHAR_lazy_if(s+1,UTF)) {
                    int len;
-                   scan_ident(s, send, tmpbuf, sizeof tmpbuf, FALSE);
+                    char *tmp = PL_bufend;
+                    PL_bufend = (char*)send;
+                    scan_ident(s, tmpbuf, sizeof tmpbuf, FALSE);
+                    PL_bufend = tmp;
                    len = (int)strlen(tmpbuf);
                    if (len > 1 && gv_fetchpvn_flags(tmpbuf, len,
                                                     UTF ? SVf_UTF8 : 0, SVt_PV)


Normally that would worry me, because a classic source of subtle bugs is to store something \(interpreter\) global in a local variable\, change it\, call something\, and restore it afterwards\. The problem being that the called thing can croak\(\)\, and the restore gets skipped\. I see that scan\_ident\(\) can croak\(\)\, but if a croak\(\) happens during parsing\, is it caught sufficiently far out that parsing is aborted\, and so the "wrong" value of PL\_bufend doesn't matter?

I've pushed that fix in the same branch, but perhaps it should go in a
different branch/ticket.

I don't know. If I understand it correctly, your first 3 commits are related,
but a refactoring, a bug fix and adding a deprecation. The last 2 commits are
related - a refactoring and then a bug fix​:

e00d033 Reworked t/base/lex.t to use less hardcoded test numbers.
12e1147 toke.c, S_scan_ident​: Skip over newlines as well as spaces and tabs.
e7ab097 [perl #119123] disallow literal control character variables
7099e60 toke.c, S_scan_ident()​: Don't take a "end of buffer" argument, use PL_bu
79406ce toke.c, scan_ident()​: use PEEKSPACE() to skip over whitespace.

But they are all dealing with the same thing, aren't they - whitespace and
similar things in variable names and {}

So they seem to fit just as well as 5 commits on the same branch, as a
3-commit and a 2-commit branch.

Additionally, with the above change, I think we could simplify
the VALID_LEN_ONE_IDENT macro from this​:
|| ((((U8)(d)) < 32)
\
&& (((((U8)(d)) >= 14)
\
|| (((U8)(d)) <= 8 && (d) != 0) \
|| (((U8)(d)) == 13))))
\
|| (((U8)(d)) == toCTRL('?')))
to this​:
|| ( isCNTRL_A((U8)d) && !isSPACE(d) )

Could you explain why? It's not obvious to me, and I guess to quite a few
other people.

Nicholas Clark

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 2, 2013

From @Hugmeir

On Mon, Sep 2, 2013 at 12​:28 PM, Nicholas Clark <nick@​ccl4.org> wrote​:

On Sun, Sep 01, 2013 at 08​:50​:38PM -0300, Brian Fraser wrote​:

On Sun, Sep 1, 2013 at 4​:53 AM, Nicholas Clark via RT <

1) cleaning up t/base/lex.t (with no functional changes)
2) fixing a long standing parse error (with changes to t/base/lex.t)
3) adding a deprecation (with changes to t/base/lex.t)

It would be lot clearer to split those 3 distinct things out.
I think it would also make sense to merge the perldelta commit that
describes the last change into the commit that makes the change.

Thank you for your review. This makes sense, apologies for the sloppy
branch! I've force-pushed a fixed version, but...

You have this​:

diff --git a/toke.c b/toke.c
index e125181..228fd12 100644
--- a/toke.c
+++ b/toke.c
@​@​ -3943,7 +3943,10 @​@​ S_intuit_more(pTHX_ char *s)
weight -= seen[un_char] * 10;
if (isWORDCHAR_lazy_if(s+1,UTF)) {
int len;
- scan_ident(s, send, tmpbuf, sizeof tmpbuf, FALSE);
+ char *tmp = PL_bufend;
+ PL_bufend = (char*)send;
+ scan_ident(s, tmpbuf, sizeof tmpbuf, FALSE);
+ PL_bufend = tmp;
len = (int)strlen(tmpbuf);
if (len > 1 && gv_fetchpvn_flags(tmpbuf, len,
UTF ? SVf_UTF8 : 0,
SVt_PV)

Normally that would worry me, because a classic source of subtle bugs is to
store something (interpreter) global in a local variable, change it, call
something, and restore it afterwards. The problem being that the called
thing
can croak(), and the restore gets skipped. I see that scan_ident() can
croak(), but if a croak() happens during parsing, is it caught sufficiently
far out that parsing is aborted, and so the "wrong" value of PL_bufend
doesn't
matter?

I think so. Mind you, I've been wrong before :) My understand is that
croak() immediately
ends parsing -- it's yyerror() that would cause this sort of bug. And since
PL_bufend is
local to the current parser, rather than the interpreter, I don't think
that it can creep out
and cause issues to the caller.

This uses yyerror()​:
$ perl -E 'eval qq{say use strict; sub foo{...}}; say $@​ ? q{Got $@​} : q{No
$@​}; say $​::{foo} ? "foo exists" : "foo does not exist"'
Got $@​
foo exists

This uses croak()​:
$ perl -E 'eval qq{\x{FFFF}; sub foo{...}}; say $@​ ? q{Got $@​} : q{No $@​};
say $​::{foo} ? "foo exists" : "foo does not exist"'
Got $@​
foo does not exist

I've pushed that fix in the same branch, but perhaps it should go in a
different branch/ticket.

I don't know. If I understand it correctly, your first 3 commits are
related,
but a refactoring, a bug fix and adding a deprecation. The last 2 commits
are
related - a refactoring and then a bug fix​:

e00d033 Reworked t/base/lex.t to use less hardcoded test numbers.
12e1147 toke.c, S_scan_ident​: Skip over newlines as well as spaces and
tabs.
e7ab097 [perl #119123] disallow literal control character variables
7099e60 toke.c, S_scan_ident()​: Don't take a "end of buffer" argument, use
PL_bu
79406ce toke.c, scan_ident()​: use PEEKSPACE() to skip over whitespace.

But they are all dealing with the same thing, aren't they - whitespace and
similar things in variable names and {}

So they seem to fit just as well as 5 commits on the same branch, as a
3-commit and a 2-commit branch.

Additionally, with the above change, I think we could simplify
the VALID_LEN_ONE_IDENT macro from this​:
|| ((((U8)(d)) < 32)
\
&& (((((U8)(d)) >= 14)
\
|| (((U8)(d)) <= 8 && (d) !=
0) \
|| (((U8)(d)) == 13))))
\
|| (((U8)(d)) == toCTRL('?')))
to this​:
|| ( isCNTRL_A((U8)d) && !isSPACE(d) )

Could you explain why? It's not obvious to me, and I guess to quite a few
other people.

Sure. 6a048a6 changed part of the definition of VALID_LEN_ONE_IDENT
from "ASCII controls" (isCNTRL_A) to "ASCII controls minus things
traditionally
matched by \s" (the large conditional above).
With the PEEKSPACE() changes in place, [\ \t\n] (and potentially \r, I
believe?)
are now being skipped, so the big conditional is now checking for "ASCII
controls
minus \f and \v (and maybe \r)".
We could either remove the checks for [\ \t\n], since they are now
superfluous,
or decrease the overall cognitive load of the conditional by simplifying it
to
isCNTRL_A && !isSPACE, but I don't know if the last bit would cause problems
for EBCDIC

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 2, 2013

From @khwilliamson

On Mon Sep 02 09​:26​:09 2013, Hugmeir wrote​:

Additionally, with the above change, I think we could simplify
the VALID_LEN_ONE_IDENT macro from this​:
|| ((((U8)(d)) < 32)
\
&& (((((U8)(d)) >= 14)
\
|| (((U8)(d)) <= 8 &&
(d) !=
0) \
|| (((U8)(d)) == 13))))
\
|| (((U8)(d)) == toCTRL('?')))
to this​:
|| ( isCNTRL_A((U8)d) &&
!isSPACE(d) )

Could you explain why? It's not obvious to me, and I guess to quite
a few
other people.

Sure. 6a048a6 changed part of the definition of VALID_LEN_ONE_IDENT
from "ASCII controls" (isCNTRL_A) to "ASCII controls minus things
traditionally
matched by \s" (the large conditional above).
With the PEEKSPACE() changes in place, [\ \t\n] (and potentially \r, I
believe?)
are now being skipped, so the big conditional is now checking for
"ASCII
controls
minus \f and \v (and maybe \r)".
We could either remove the checks for [\ \t\n], since they are now
superfluous,
or decrease the overall cognitive load of the conditional by
simplifying it
to
isCNTRL_A && !isSPACE, but I don't know if the last bit would cause
problems
for EBCDIC

The reason for that cognitive load is entirely EBCDIC. It isn't that
these variable names match the ASCII control characters, it is that they
match \cA, \cB, etc, The set are those that evaluate to the numbers 1-32
excluding a few, plus \c?. So, unfortunately that can't be simplified
as hugmeir suggests.
--
Karl Williamson

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 15, 2013

From @Hugmeir

On Mon, Sep 2, 2013 at 2​:44 PM, Karl Williamson via RT <
perlbug-followup@​perl.org> wrote​:

On Mon Sep 02 09​:26​:09 2013, Hugmeir wrote​:

Additionally, with the above change, I think we could simplify
the VALID_LEN_ONE_IDENT macro from this​:
|| ((((U8)(d)) < 32)
\
&& (((((U8)(d)) >= 14)
\
|| (((U8)(d)) <= 8 &&
(d) !=
0) \
|| (((U8)(d)) == 13))))
\
|| (((U8)(d)) == toCTRL('?')))
to this​:
|| ( isCNTRL_A((U8)d) &&
!isSPACE(d) )

Could you explain why? It's not obvious to me, and I guess to quite
a few
other people.

Sure. 6a048a6 changed part of the definition of VALID_LEN_ONE_IDENT
from "ASCII controls" (isCNTRL_A) to "ASCII controls minus things
traditionally
matched by \s" (the large conditional above).
With the PEEKSPACE() changes in place, [\ \t\n] (and potentially \r, I
believe?)
are now being skipped, so the big conditional is now checking for
"ASCII
controls
minus \f and \v (and maybe \r)".
We could either remove the checks for [\ \t\n], since they are now
superfluous,
or decrease the overall cognitive load of the conditional by
simplifying it
to
isCNTRL_A && !isSPACE, but I don't know if the last bit would cause
problems
for EBCDIC

The reason for that cognitive load is entirely EBCDIC. It isn't that
these variable names match the ASCII control characters, it is that they
match \cA, \cB, etc, The set are those that evaluate to the numbers 1-32
excluding a few, plus \c?. So, unfortunately that can't be simplified
as hugmeir suggests.
--
Karl Williamson

Aw, that's a shame. Thank you for the feedback.

That aside, if there are no objections with the branch as-is, there's a
shiny new commit bit I'd like on it : D

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 15, 2013

From @cpansprout

On Sat Sep 14 19​:24​:17 2013, Hugmeir wrote​:

That aside, if there are no objections with the branch as-is,

One question​:

- while (s < PL_bufend && ( SPACE_OR_TAB(*s) || *s == '\n' ))
- s++;
+ orig_copline = CopLINE(PL_curcop);
+ while (s < PL_bufend && isSPACE(*s)) {
+ s = PEEKSPACE(s);
+ }

Is while necessary there? Won’t if work?

there's a
shiny new commit bit I'd like on it : D

Congratulations! A week from now, mine will be three years old. Are
they always given out in September? :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 15, 2013

From @Hugmeir

On Sun, Sep 15, 2013 at 2​:42 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Sep 14 19​:24​:17 2013, Hugmeir wrote​:

That aside, if there are no objections with the branch as-is,

One question​:

- while (s < PL_bufend && ( SPACE_OR_TAB(*s) || *s == '\n' ))
- s++;
+ orig_copline = CopLINE(PL_curcop);
+ while (s < PL_bufend && isSPACE(*s)) {
+ s = PEEKSPACE(s);
+ }

Is while necessary there? Won’t if work?

It's not, cargo-culted that from the previous version; 'if' will work well.
Nice catch!

there's a
shiny new commit bit I'd like on it : D

Congratulations! A week from now, mine will be three years old. Are
they always given out in September? :-)

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119123

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 12, 2013

From @jkeenan

On Sun Sep 15 12​:05​:22 2013, Hugmeir wrote​:

On Sun, Sep 15, 2013 at 2​:42 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Sep 14 19​:24​:17 2013, Hugmeir wrote​:

That aside, if there are no objections with the branch as-is,

One question​:

- while (s < PL_bufend && ( SPACE_OR_TAB(*s) || *s == '\n' ))
- s++;
+ orig_copline = CopLINE(PL_curcop);
+ while (s < PL_bufend && isSPACE(*s)) {
+ s = PEEKSPACE(s);
+ }

Is while necessary there? Won’t if work?

It's not, cargo-culted that from the previous version; 'if' will work well.
Nice catch!

there's a
shiny new commit bit I'd like on it : D

Congratulations! A week from now, mine will be three years old. Are
they always given out in September? :-)

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119123

Father C, hugmeir, khw​: Can we get an update on the status of issues discussed in this ticket?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 13, 2013

From @Hugmeir

On Wed, Dec 11, 2013 at 11​:27 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Sun Sep 15 12​:05​:22 2013, Hugmeir wrote​:

On Sun, Sep 15, 2013 at 2​:42 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Sat Sep 14 19​:24​:17 2013, Hugmeir wrote​:

That aside, if there are no objections with the branch as-is,

One question​:

- while (s < PL_bufend && ( SPACE_OR_TAB(*s) || *s == '\n' ))
- s++;
+ orig_copline = CopLINE(PL_curcop);
+ while (s < PL_bufend && isSPACE(*s)) {
+ s = PEEKSPACE(s);
+ }

Is while necessary there? Won’t if work?

It's not, cargo-culted that from the previous version; 'if' will work
well.
Nice catch!

there's a
shiny new commit bit I'd like on it : D

Congratulations! A week from now, mine will be three years old. Are
they always given out in September? :-)

--

Father Chrysostomos

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119123

Father C, hugmeir, khw​: Can we get an update on the status of issues
discussed in this ticket?

Ah, thanks for following through! I forgot to mark this as resolved; The
fix was merged as b29f65f (plus some
related fixes in the previous and following commits).

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 13, 2013

From @Hugmeir

Marking as resolved.

--hugmeir

@p5pRT p5pRT closed this Dec 13, 2013
@p5pRT
Copy link
Author

@p5pRT p5pRT commented Dec 13, 2013

@Hugmeir - Status changed from 'open' to 'resolved'

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