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

Regression after +@ → nqp::elems(@) optimization #1922

Open
AlexDaniel opened this issue Jun 14, 2018 · 3 comments
Open

Regression after +@ → nqp::elems(@) optimization #1922

AlexDaniel opened this issue Jun 14, 2018 · 3 comments
Labels
NQP regression Issue did not exist previously

Comments

@AlexDaniel
Copy link
Contributor

Grammar::BNF module fails on abnf.t test file with compile time error:

===SORRY!===
This type (Array) does not support elems

Bisected: (2018-06-10) c10b1ee

Looking at the bump, I think this is the offending commit: Raku/nqp@efb316e

Running with --ll-exception:

This type (Array) does not support elems
   at src/Perl6/World.nqp:1380  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/World.moarvm:install_package)
 from /home/alex/git/Grammar-BNF/lib/Slang/ABNF.pm (Slang::ABNF):39  (/home/alex/git/Grammar-BNF/lib/.precomp/0344DA70E613FAAC3349CDE45ED142296B7BDC0B/86/86C4D4DBA4A40BC9F9622C96275D9BE48ADB55F3:package_declarator:sym<abnf-grammar>)
 from gen/moar/stage2/QRegex.nqp:1651  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QRegex.moarvm:!reduce)
 from gen/moar/stage2/QRegex.nqp:1594  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QRegex.moarvm:!cursor_pass)
 from /home/alex/git/Grammar-BNF/lib/Slang/ABNF.pm (Slang::ABNF):19  (/home/alex/git/Grammar-BNF/lib/.precomp/0344DA70E613FAAC3349CDE45ED142296B7BDC0B/86/86C4D4DBA4A40BC9F9622C96275D9BE48ADB55F3:package_declarator:sym<abnf-grammar>)
 from gen/moar/stage2/QRegex.nqp:1696  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QRegex.moarvm:!protoregex)
 from <unknown>:1  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:package_declarator)
 from <unknown>:1  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:term:sym<package_declarator>)
 from gen/moar/stage2/QRegex.nqp:1696  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QRegex.moarvm:!protoregex)
 from <unknown>:1  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:term)
 from gen/moar/Perl6-Grammar.nqp:4018  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:termish)
 from gen/moar/stage2/NQPHLL.nqp:877  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:EXPR)
 from gen/moar/Perl6-Grammar.nqp:4058  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:EXPR)
 from gen/moar/Perl6-Grammar.nqp:1352  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:statement)
 from gen/moar/Perl6-Grammar.nqp:1280  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:statementlist)
 from gen/moar/stage2/NQPHLL.nqp:1107  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:LANG)
 from gen/moar/Perl6-Grammar.nqp:1727  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:FOREIGN_LANG)
 from gen/moar/Perl6-Grammar.nqp:1244  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:comp_unit)
 from gen/moar/Perl6-Grammar.nqp:545  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:TOP)
 from gen/moar/stage2/QRegex.nqp:2303  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QRegex.moarvm:parse)
 from gen/moar/stage2/NQPHLL.nqp:1877  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:parse)
 from gen/moar/stage2/NQPHLL.nqp:1793  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:execute_stage)
 from gen/moar/stage2/NQPHLL.nqp:1826  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:run)
 from gen/moar/stage2/NQPHLL.nqp:1829  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:)
 from gen/moar/stage2/NQPHLL.nqp:1815  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:compile)
 from gen/moar/stage2/NQPHLL.nqp:1513  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:eval)
 from gen/moar/stage2/NQPHLL.nqp:1770  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:evalfiles)
 from gen/moar/stage2/NQPHLL.nqp:1693  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:command_eval)
 from src/Perl6/Compiler.nqp:42  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Compiler.moarvm:command_eval)
 from gen/moar/stage2/NQPHLL.nqp:1619  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:command_line)
 from gen/moar/main.nqp:47  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/perl6.moarvm:MAIN)
 from gen/moar/main.nqp:38  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/perl6.moarvm:<mainline>)
 from <unknown>:1  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/perl6.moarvm:<main>)
 from <unknown>:1  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/perl6.moarvm:<entry>)

I don't have a golf of the issue yet.

@AlexDaniel AlexDaniel added regression Issue did not exist previously BLOCKER Preventing the next release of rakudo, or just needing attention before the release labels Jun 14, 2018
@jnthn
Copy link
Member

jnthn commented Jun 16, 2018

The optimization is legitimate so far as NQP code goes, but relies on an @ and % sigil thing in NQP holding a real NQP array. Here, a Perl 6 array seems to make it into the code, thanks to the slang being written in Perl 6, and thus things don't work out. That appears to happen here.

The larger problem here is that some (most?) slangs are relying on compiler details that we don't consider a supported API. This situation isn't going to improve until we do the QTree work as part of macros and provide a supported one, which it goes without saying is a big effort. While I'm fairly optimistic we'll find a way to do it step by step, the end result will effectively amount to a rewrite of Perl6::Actions, Perl6::World, and parts of Perl6::Grammar. I'm hugely doubtful we can pull it off without breaking a significant number of existing slangs. So we're going to have an amount of pain in this area in the future. That said, I think we should make reasonable efforts to not break such modules until then.

In the immediate, I think we can:

  • Revert the NQP change for this release
  • For the next release, see if we can switch to using a $ sigil in that place in Perl6::World, so the optimization doesn't take effect there, and so we don't break this module for now.

That's a quite localized change, so feels reasonable.

AlexDaniel referenced this issue in Raku/nqp Jun 16, 2018
This reverts commit efb316e.

See #1922 for more info.
@AlexDaniel
Copy link
Contributor Author

Reverted, leaving open for further discussion.

@AlexDaniel AlexDaniel removed the BLOCKER Preventing the next release of rakudo, or just needing attention before the release label Jun 16, 2018
AlexDaniel added a commit that referenced this issue Jun 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NQP regression Issue did not exist previously
Projects
None yet
Development

No branches or pull requests

3 participants