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

Unspace after a sigil-less term causes parser error #5396

Closed
p6rt opened this issue Jun 22, 2016 · 9 comments
Closed

Unspace after a sigil-less term causes parser error #5396

p6rt opened this issue Jun 22, 2016 · 9 comments
Labels

Comments

@p6rt
Copy link

@p6rt p6rt commented Jun 22, 2016

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

Searchable as RT128462$

@p6rt
Copy link
Author

@p6rt p6rt commented Jun 22, 2016

From @smls

  my \term = 42;
  say term.Str; # Works fine
  say term\ .Str; # Error​: "Variable '&term' is not declared"

@p6rt
Copy link
Author

@p6rt p6rt commented Jun 22, 2016

From @lizmat

On 22 Jun 2016, at 15​:46, Sam S. (via RT) <perl6-bugs-followup@​perl.org> wrote​:

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

my \term = 42;
say term.Str; # Works fine
say term\ .Str; # Error​: "Variable '&term' is not declared"

Thanks for the report.

Please note that you don’t need to unspace anymore in the given case​:

  say term .Str

just works since just before Christmas :-)

Liz

@p6rt
Copy link
Author

@p6rt p6rt commented Jun 22, 2016

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

@p6rt
Copy link
Author

@p6rt p6rt commented Jul 12, 2016

From @zoffixznet

Fudged tests added in Raku/roast@44fc047

@p6rt
Copy link
Author

@p6rt p6rt commented Jul 28, 2016

From @lizmat

On 22 Jun 2016, at 15​:46, Sam S. (via RT) <perl6-bugs-followup@​perl.org> wrote​:

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

my \term = 42;
say term.Str; # Works fine
say term\ .Str; # Error​: "Variable '&term' is not declared"

FWIW, it looks like a problem in the Optimizer of all places​:

$ 6l 'my \a = 42; say a\ .Str'
Variable '&a' is not declared
  at gen/moar/m-CORE.setting​:21915 (/Users/liz/Github/rakudo.moar/install/share/perl6/runtime/./CORE.setting.moarvm​:throw)
from gen/moar/m-Perl6-Optimizer.nqp​:379 (blib/Perl6/Optimizer.moarvm​:report)
from gen/moar/m-Perl6-Optimizer.nqp​:894 (blib/Perl6/Optimizer.moarvm​:optimize)
from src/Perl6/Compiler.nqp​:32 (blib/Perl6/Compiler.moarvm​:optimize)
from gen/moar/stage2/NQPHLL.nqp​:1751 (/Users/liz/Github/rakudo.moar/install/share/nqp/lib/NQPHLL.moarvm​:compile)
from gen/moar/stage2/NQPHLL.nqp​:1487 (/Users/liz/Github/rakudo.moar/install/share/nqp/lib/NQPHLL.moarvm​:eval)
from gen/moar/stage2/NQPHLL.nqp​:1595 (/Users/liz/Github/rakudo.moar/install/share/nqp/lib/NQPHLL.moarvm​:)
from gen/moar/stage2/NQPHLL.nqp​:1592 (/Users/liz/Github/rakudo.moar/install/share/nqp/lib/NQPHLL.moarvm​:command_eval)
from src/Perl6/Compiler.nqp​:27 (blib/Perl6/Compiler.moarvm​:command_eval)
from gen/moar/stage2/NQPHLL.nqp​:1576 (/Users/liz/Github/rakudo.moar/install/share/nqp/lib/NQPHLL.moarvm​:command_line)
from gen/moar/m-main.nqp​:37 (/Users/liz/Github/rakudo.moar/install/share/perl6/runtime/perl6.moarvm​:MAIN)
from gen/moar/m-main.nqp​:33 (/Users/liz/Github/rakudo.moar/install/share/perl6/runtime/perl6.moarvm​:<mainline>)
from <unknown>​:1 (/Users/liz/Github/rakudo.moar/install/share/perl6/runtime/perl6.moarvm​:<main>)
from <unknown>​:1 (/Users/liz/Github/rakudo.moar/install/share/perl6/runtime/perl6.moarvm​:<entry>)

@p6rt
Copy link
Author

@p6rt p6rt commented May 28, 2017

From @smls

This bug is still present in

  This is Rakudo version 2017.05-272-gfa7aa1c36 built on MoarVM version 2017.05-25-g62bc54e9
  implementing Perl 6.c.

@p6rt
Copy link
Author

@p6rt p6rt commented Sep 30, 2017

From @skids

On Sun, 28 May 2017 02​:31​:34 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-272-gfa7aa1c36 built on MoarVM version
2017.05-25-g62bc54e9
implementing Perl 6.c.

Not an optimizer problem, still happens with --optimize=0

Here's a history of the rule term​:sym<identifier>, which apparently
only handles the case of subroutine calls of the form "thing()".

We start here many years ago​:
<identifier> <?[(]> <args>

Then we decide we do not want typecasts to fall into this rule​:
<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?[(]> <args>

Then we implement unspace between the name and the parenthesis.
<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?before <.unsp>|'('> <args>

...that is a bit tricky... was it intentionally not "<.unsp>? '('"?
We'll get into that in a bit.

Next we get a commit that does some better error reporting (there's more
before and after the part we are watching)

<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?before <.unsp>|'('> <![​:]> { $pos := $/.pos } <args(1)>

...so, what's that new "<![​:]>" for? A '​:' cannot happen here
due to the "before"...

So, let's look at <args>, keeping in mind that it can
only happen after "foo\ " or "foo ("... but the unspace
or paren in those two forms is part of the string <args>
is matching.

There's other stuff in it but this is the meat​:

  [
  | '(' ~ ')' <semiarglist>
  | <.unsp> '(' ~ ')' <semiarglist>
  | [ \s <arglist> ]
  | <?>
  ]

...all four cases can hit. Only the first two were hittable
before the unspace change... well maybe a malformed semiarglist
could get into the fourth one somehow.

The unspace change would prevent the third one from being hit...
unless there were a way to follow an unspace by a space... but it would
be silly to only then accept a single space character there.
One wonders what nefarious corner case that covers.

The real problem comes with that fourth alternative. We of
course need to support calling with no parens for argumentless
forms. But that case can fall through to either of term​:sym<name>
or term​:sym<identfier>. Both actions methods seem to be able to
handle setting up barename calls.

But you can fall through this case to term​::sym<identifier> even
when there is something else after the unspace which does not
resemble an argument list at all. Only term​:sym<name> has the
ability to handle that. Like, in this ticket, the rest
of a postfix method call. That means this​:

a\ .Str

...is being processed as​:

(&a()) $_.Str

Now, maybe LTM was being leaned on to force that case through
term​:sym<name>, but perhaps some of the error reporting stuff
made some confusing sequenceish points that ruined that. At
any rate this weekend I'll submit a patch changing it to​:

<identifier> <!{ $*W.is_type([~$<identifier>]) }> [ <?before <.unsp>? '('> <?> | \\ <?before '('> ]

...which passes spectest and handles these cases​:

$ perl6 -e 'my constant \a = 42; say a\ .Str;'
42
$ perl6 -e 'say\(42)'
42
$ perl6 -e 'my constant \a = [1,2]; say a\ [1];'
2
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\ {"b"};'
True
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\ <b>;'
True

...and I'll see if I can find out what else to tweak to make
this work​:

$ perl6 -e 'class A { our sub foo ($a) { "OHAI $a".say } }; A​::foo\(42)'
Too few positionals passed; expected 1 argument but got 0
  in sub foo at -e line 1
  in block <unit> at -e line 1

...it looks like this patch breaks the following, but I'm not sure
that it should work​:

$ perl6 -e 'say\ 42'
===SORRY!===
Argument to "say" seems to be malformed

...and these all already worked, but I'll make sure they are tested
when I write the tests​:

$ perl6 -e 'say\ (42)'
42
$ perl6 -e 'my constant \a = [1,2]; say a\[1];'
2
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\{"b"};'
True
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\<b>;'
True
$ perl6 -e 'class A { our sub foo ($a) { "OHAI $a".say } }; A​::foo\ (42)'
OHAI 42
$ perl6 -e 'say IO​::Path\ .^name;'
IO​::Path
$ perl6 -e 'IO​::Path\.^name.say'
IO​::Path
$ perl6 -e 'class A { our @​a = 1,2; }; say @​A​::a\ [1]'
2
$ perl6 -e 'class A { our @​a = 1,2; }; say @​A​::a\[1]'
2
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\{"b"}'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\ {"b"}'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\ <b>'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\<b>'
True

@p6rt
Copy link
Author

@p6rt p6rt commented Oct 1, 2017

From @skids

On Fri, 29 Sep 2017 22​:43​:56 -0700, bri@​abrij.org wrote​:

On Sun, 28 May 2017 02​:31​:34 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-272-gfa7aa1c36 built on MoarVM version
2017.05-25-g62bc54e9
implementing Perl 6.c.

Not an optimizer problem, still happens with --optimize=0

Here's a history of the rule term​:sym<identifier>, which apparently
only handles the case of subroutine calls of the form "thing()".

We start here many years ago​:
<identifier> <?[(]> <args>

Then we decide we do not want typecasts to fall into this rule​:
<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?[(]> <args>

Then we implement unspace between the name and the parenthesis.
<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?before
<.unsp>|'('> <args>

...that is a bit tricky... was it intentionally not "<.unsp>? '('"?
We'll get into that in a bit.

Next we get a commit that does some better error reporting (there's
more
before and after the part we are watching)

<identifier> <!{ $*W.is_type([~$<identifier>]) }> <?before
<.unsp>|'('> <![​:]> { $pos := $/.pos } <args(1)>

...so, what's that new "<![​:]>" for? A '​:' cannot happen here
due to the "before"...

So, let's look at <args>, keeping in mind that it can
only happen after "foo\ " or "foo ("... but the unspace
or paren in those two forms is part of the string <args>
is matching.

There's other stuff in it but this is the meat​:

[
| '(' ~ ')' <semiarglist>
| <.unsp> '(' ~ ')' <semiarglist>
| [ \s <arglist> ]
| <?>
]

...all four cases can hit. Only the first two were hittable
before the unspace change... well maybe a malformed semiarglist
could get into the fourth one somehow.

The unspace change would prevent the third one from being hit...
unless there were a way to follow an unspace by a space... but it
would
be silly to only then accept a single space character there.
One wonders what nefarious corner case that covers.

The real problem comes with that fourth alternative. We of
course need to support calling with no parens for argumentless
forms. But that case can fall through to either of term​:sym<name>
or term​:sym<identfier>. Both actions methods seem to be able to
handle setting up barename calls.

But you can fall through this case to term​::sym<identifier> even
when there is something else after the unspace which does not
resemble an argument list at all. Only term​:sym<name> has the
ability to handle that. Like, in this ticket, the rest
of a postfix method call. That means this​:

a\ .Str

...is being processed as​:

(&a()) $_.Str

Now, maybe LTM was being leaned on to force that case through
term​:sym<name>, but perhaps some of the error reporting stuff
made some confusing sequenceish points that ruined that. At
any rate this weekend I'll submit a patch changing it to​:

<identifier> <!{ $*W.is_type([~$<identifier>]) }> [ <?before <.unsp>?
'('> <?> | \\ <?before '('> ]

...which passes spectest and handles these cases​:

$ perl6 -e 'my constant \a = 42; say a\ .Str;'
42
$ perl6 -e 'say\(42)'
42
$ perl6 -e 'my constant \a = [1,2]; say a\ [1];'
2
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\ {"b"};'
True
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\ <b>;'
True

...and I'll see if I can find out what else to tweak to make
this work​:

$ perl6 -e 'class A { our sub foo ($a) { "OHAI $a".say } };
A​::foo\(42)'
Too few positionals passed; expected 1 argument but got 0
in sub foo at -e line 1
in block <unit> at -e line 1

...it looks like this patch breaks the following, but I'm not sure
that it should work​:

$ perl6 -e 'say\ 42'
===SORRY!===
Argument to "say" seems to be malformed

...and these all already worked, but I'll make sure they are tested
when I write the tests​:

$ perl6 -e 'say\ (42)'
42
$ perl6 -e 'my constant \a = [1,2]; say a\[1];'
2
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\{"b"};'
True
$ perl6 -e 'my constant \a = {​:a,​:b}; say a\<b>;'
True
$ perl6 -e 'class A { our sub foo ($a) { "OHAI $a".say } }; A​::foo\
(42)'
OHAI 42
$ perl6 -e 'say IO​::Path\ .^name;'
IO​::Path
$ perl6 -e 'IO​::Path\.^name.say'
IO​::Path
$ perl6 -e 'class A { our @​a = 1,2; }; say @​A​::a\ [1]'
2
$ perl6 -e 'class A { our @​a = 1,2; }; say @​A​::a\[1]'
2
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\{"b"}'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\ {"b"}'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\ <b>'
True
$ perl6 -e 'class A { our %a = :a,​:b; }; say %A​::a\<b>'
True

Fix added in rakudo commit f427575432 and additional tests in roast c22107d53. Resolving this ticket.

@p6rt p6rt closed this Oct 1, 2017
@p6rt
Copy link
Author

@p6rt p6rt commented Oct 1, 2017

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

@p6rt p6rt added the parser label Jan 5, 2020
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
You can’t perform that action at this time.