From @cpansprout

In ticket #122661, Yves Orton writes​:

perl -le'map { no warnings; $x++ } 1..10'
"no" not allowed in expression at -e line 1, at end of line
BEGIN not safe after errors--compilation aborted at -e line 1.

That is the parser thinks the open { is the beginning of a hash constructor.

This works​:

perl -le'map {; no warnings; $x++ } 1..10'

because the {; tells the parser early enough "this can't be a hash constructor".

Personally I would love to see a "once and for all" fix to this class of stupid.

The parser is setting its ‘expectation’ (PL_expect) to XTERM (expecting a term) right after the opening brace.

We could fix this ‘"no" not allowed in expression’ several ways, each having its own drawbacks.

1) Just remove that particular check in the lexer that prevents ‘use’ or ‘no’ from being used in an expression, as the parser has no problem with it. But the error message becomes less helpful​:

before$ ./miniperl -e '1 + use strict'
"use" not allowed in expression at -e line 1, near "+ "
syntax error at -e line 1, near "+ use strict
"
Execution of -e aborted due to compilation errors.
after$ ./miniperl -e '1 + use strict'
syntax error at -e line 1, near "+ use strict
"
Execution of -e aborted due to compilation errors.

2) Simply change the expectation to XSTATE (expecting a statement)

That would break any code doing returning a list of hash refs like this​:

  map {{ $foo, $_ }} ...

3) Set PL_expect to XSTATE unless there is an opening brace.

While this would take care of map {{ .... }} and map {no ...}. But what about map{BEGIN...}? The current behaviour is to treat BEGIN as not-a-keyword unless it is at the beginning of a statement (more precisely, unless PL_expect is XSTATE). Is it acceptable to break that?

$ ./perl -Ilib -le 'print for map {BEGIN . $_} 3..5'
BEGIN3
BEGIN4
BEGIN5

Also, setting PL_expect to XSTATE will allow pod where we did not allow it before​:

map {
=cut
blah blah blah
=cut
{...} }

but it would be very hard to see through the pod to detect the opening brace (based on the current pod implementation, though I am wondering whether changing the implementation to make this work would actually simplify it). And presumably that example should behave the same way as this​:

map {
# a multiline
# comment that actually
# consists of multiple
# single-line comments
{...} }

Or at least one might expect it to.

I suppose we have a fourth option​:

4) Set PL_expect to XSTATE if the opening brace is followed by ‘use’ or ‘no’ or ‘CORE​::use’ or ‘CORE​::no’.

BTW, the same code paths are used for print{...} ${...} and map{...}, so any changes would affect all three, unless we put it special exceptions. Also, I imagine that PL_expect is currently XTERM here to make ${{...}}{...} work as one would expect.

Numbers 2 and 3 would also affect other things​:

map { ...; } @​_ # yes, this would work

map { foo​: goto foo if !rand } @​_; # labels currently forbidden here

This discrepancy in error messages would go away​:

$ ./perl -Ilib -le 'for our *a (1..10) {$_.=$x}'
Missing $ on loop variable at -e line 1.
$ ./perl -Ilib -le 'map{for our *a (1..10) {$_.=$x}}'
syntax error at -e line 1, near "our *a"
Execution of -e aborted due to compilation errors.

--

Father Chrysostomos

From @ikegami

On Mon, Sep 15, 2014 at 1​:53 AM, Father Chrysostomos <
perlbug-followup@​perl.org> wrote​:

2) Simply change the expectation to XSTATE (expecting a statement)

That would break any code doing returning a list of hash refs like this​:

map {{ $foo, $_ }} ...

3) Set PL_expect to XSTATE unless there is an opening brace.

#3 fixes C<< map {{ foo => $_ }} @​a >>, but #2 and #3 still break the
equivalent C<< map EXPR, LIST >> version, C<< map { foo => $_ }, @​a >>

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

From @cpansprout

On Mon Sep 15 08​:20​:36 2014, ikegami@​adaelis.com wrote​:

On Mon, Sep 15, 2014 at 1​:53 AM, Father Chrysostomos <
perlbug-followup@​perl.org> wrote​:

2) Simply change the expectation to XSTATE (expecting a statement)

That would break any code doing returning a list of hash refs like this​:

map {{ $foo, $_ }} ...

3) Set PL_expect to XSTATE unless there is an opening brace.

#3 fixes C<< map {{ foo => $_ }} @​a >>, but #2 and #3 still break the
equivalent C<< map EXPR, LIST >> version, C<< map { foo => $_ }, @​a >>

I consider that a separate issue. What this ticket is about is​: How do we parse the contents when the lexer has already decided that we have a block here?

--

Father Chrysostomos

From @ikegami

On Mon, Sep 15, 2014 at 11​:24 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Mon Sep 15 08​:20​:36 2014, ikegami@​adaelis.com wrote​:

On Mon, Sep 15, 2014 at 1​:53 AM, Father Chrysostomos <
perlbug-followup@​perl.org> wrote​:

2) Simply change the expectation to XSTATE (expecting a statement)

That would break any code doing returning a list of hash refs like
this​:

map {{ $foo, $_ }} ...

3) Set PL_expect to XSTATE unless there is an opening brace.

#3 fixes C<< map {{ foo => $_ }} @​a >>, but #2 and #3 still break the
equivalent C<< map EXPR, LIST >> version, C<< map { foo => $_ }, @​a >>

I consider that a separate issue. What this ticket is about is​: How do we
parse the contents when the lexer has already decided that we have a block
here?

I thought C<< map { no warnings; ... } LIST >> failed because Perl treated
the { } as a hash constructor.

From @cpansprout

On Mon Sep 15 09​:38​:06 2014, ikegami@​adaelis.com wrote​:

On Mon, Sep 15, 2014 at 11​:24 AM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

On Mon Sep 15 08​:20​:36 2014, ikegami@​adaelis.com wrote​:

On Mon, Sep 15, 2014 at 1​:53 AM, Father Chrysostomos <
perlbug-followup@​perl.org> wrote​:

2) Simply change the expectation to XSTATE (expecting a statement)

That would break any code doing returning a list of hash refs like
this​:

map {{ $foo, $_ }} ...

3) Set PL_expect to XSTATE unless there is an opening brace.

#3 fixes C<< map {{ foo => $_ }} @​a >>, but #2 and #3 still break the
equivalent C<< map EXPR, LIST >> version, C<< map { foo => $_ }, @​a >>

I consider that a separate issue. What this ticket is about is​: How do we
parse the contents when the lexer has already decided that we have a block
here?

I thought C<< map { no warnings; ... } LIST >> failed because Perl treated
the { } as a hash constructor.

No, that’s not why. I’ve trimmed the output of the following. The last line shown is the important one. Notice that one has '{' while the other has HASHBRACK​:

$ ./miniperl -DT -e 'map { no warnings;'
### 0​:LEX_NORMAL/XSTATE "\n"
### <== LSTOP(ival=op_mapstart)

### 1​:LEX_NORMAL/XREF " { no warnings;\n"
### <== '{'

$ ./miniperl -DT -e 'map { no => warnings;'
### 0​:LEX_NORMAL/XSTATE "\n"
### <== LSTOP(ival=op_mapstart)

### 1​:LEX_NORMAL/XREF " { no => warnings;\n"
### <== HASHBRACK

--

Father Chrysostomos

From @ikegami

On Mon, Sep 15, 2014 at 12​:48 PM, Father Chrysostomos via RT <
perlbug-followup@​perl.org> wrote​:

No, that’s not why. I’ve trimmed the output of the following. The last
line shown is the important one. Notice that one has '{' while the other
has HASHBRACK​:

Thanks.

From @cpansprout

On Sun Sep 14 22​:53​:04 2014, sprout wrote​:

While this would take care of map {{ .... }} and map {no ...}. But
what about map{BEGIN...}? The current behaviour is to treat BEGIN as
not-a-keyword unless it is at the beginning of a statement (more
precisely, unless PL_expect is XSTATE). Is it acceptable to break
that?

$ ./perl -Ilib -le 'print for map {BEGIN . $_} 3..5'
BEGIN3
BEGIN4
BEGIN5

Nothing on CPAN matches /(printf?|say|map|grep)\s*\{\s*(BEGIN|AUTOLOAD|UNITCHECK|DESTROY|END|INIT|CHECK)\b/, nor can I think of any real use for this, so I think it is acceptable.

Also, setting PL_expect to XSTATE will allow pod where we did not
allow it before​:

map {
=cut
blah blah blah
=cut
{...} }

but it would be very hard to see through the pod to detect the opening
brace (based on the current pod implementation, though I am wondering
whether changing the implementation to make this work would actually
simplify it). And presumably that example should behave the same way
as this​:

map {
# a multiline
# comment that actually
# consists of multiple
# single-line comments
{...} }

Or at least one might expect it to.

Or maybe not, since pod is only allowed at the beginning of a statement, so it currently is not treated as just whitespace.

--

Father Chrysostomos

From @cpansprout

On Sun Sep 14 22​:53​:04 2014, sprout wrote​:

Numbers 2 and 3 would also affect other things​:
...
map { foo​: goto foo if !rand } @​_; # labels currently forbidden here

Formerly, ${foo​:fit} would parse that as a simple variable named ‘foo​:fit’ in the current package. That was fixed in 5.18. Allowing ${no strict; ...} and map {no strict; ...} will allow ${foo​:fit} once more, but with a different meaning from before. Now it will mean the same thing that ${; foo​: fit} currently means; i.e., ‘foo’ is a statement label, and ‘fit’ is a bareword or sub call returning a scalar reference or variable name.

Is that change acceptable? I think it is. It did the wrong thing, until it was made an error for two stable releases, and then it becomes more consistent with the rest of the language.

--

Father Chrysostomos

From @cpansprout

On Sun Sep 14 22​:53​:04 2014, sprout wrote​:

In ticket #122661, Yves Orton writes​:

perl -le'map { no warnings; $x++ } 1..10'
"no" not allowed in expression at -e line 1, at end of line
BEGIN not safe after errors--compilation aborted at -e line 1.

That is the parser thinks the open { is the beginning of a hash
constructor.

This works​:

perl -le'map {; no warnings; $x++ } 1..10'

because the {; tells the parser early enough "this can't be a hash
constructor".

Personally I would love to see a "once and for all" fix to this class
of stupid.

The parser is setting its ‘expectation’ (PL_expect) to XTERM
(expecting a term) right after the opening brace.

We could fix this ‘"no" not allowed in expression’ several ways, each
having its own drawbacks.
...
3) Set PL_expect to XSTATE unless there is an opening brace.

I have fixed this bug in e660c40, following a variant of this option. It is not only the opening brace that is considered, but also ‘sub​:’ to allow &{sub​:lvalue{}} to continue to dwim.

So the following changes from my original post have occurred, too​:

While this would take care of map {{ .... }} and map {no ...}. But
what about map{BEGIN...}? The current behaviour is to treat BEGIN as
not-a-keyword unless it is at the beginning of a statement (more
precisely, unless PL_expect is XSTATE). Is it acceptable to break
that?

$ ./perl -Ilib -le 'print for map {BEGIN . $_} 3..5'
BEGIN3
BEGIN4
BEGIN5

Also, setting PL_expect to XSTATE will allow pod where we did not
allow it before​:

map {
=cut
blah blah blah
=cut
{...} }

...

map { ...; } @​_ # yes, this would work

map { foo​: goto foo if !rand } @​_; # labels currently forbidden here

This discrepancy in error messages would go away​:

$ ./perl -Ilib -le 'for our *a (1..10) {$_.=$x}'
Missing $ on loop variable at -e line 1.
$ ./perl -Ilib -le 'map{for our *a (1..10) {$_.=$x}}'
syntax error at -e line 1, near "our *a"
Execution of -e aborted due to compilation errors.

--

Father Chrysostomos

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

From @rjbs

* Father Chrysostomos via RT <perlbug-followup@​perl.org> [2014-10-27T12​:25​:45]

Is that change acceptable? I think it is. It did the wrong thing, until it
was made an error for two stable releases, and then it becomes more
consistent with the rest of the language.

For the record​: yes, I think so.

--
rjbs