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

“MVMHash representation requires MVMString keys” rescalar regression #2056

Open
AlexDaniel opened this Issue Jul 12, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@AlexDaniel
Member

AlexDaniel commented Jul 12, 2018

Bisected: (2018-07-09) d3c5381

Running this:

PERL6LIB=lib/ perl6 t/100-precis.t

This is the result:

===SORRY!===
MVMHash representation requires MVMString keys

With --ll-exception:

MVMHash representation requires MVMString keys
   at gen/moar/stage2/QAST.nqp:6755  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/QAST.moarvm:assemble_to_file)
 from gen/moar/stage2/NQPHLL.nqp:443  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:mbc)
 from gen/moar/stage2/NQPHLL.nqp:1825  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:execute_stage)
 from gen/moar/stage2/NQPHLL.nqp:1861  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:run)
 from gen/moar/stage2/NQPHLL.nqp:1864  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:)
 from gen/moar/stage2/NQPHLL.nqp:1850  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:compile)
 from gen/moar/stage2/NQPHLL.nqp:1548  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:eval)
 from gen/moar/stage2/NQPHLL.nqp:1805  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:evalfiles)
 from gen/moar/stage2/NQPHLL.nqp:1728  (/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:1654  (/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>)

   at SETTING::src/core/Exception.pm6:57  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:throw)
 from SETTING::src/core/control.pm6:178  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:die)
 from SETTING::src/core/control.pm6:166  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:die)
 from SETTING::src/core/CompUnit/PrecompilationRepository.pm6:293  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:precompile)
 from SETTING::src/core/CompUnit/PrecompilationRepository.pm6:210  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:precompile)
 from SETTING::src/core/CompUnit/PrecompilationRepository.pm6:46  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:try-load)
 from SETTING::src/core/CompUnit/Repository/FileSystem.pm6:135  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:)
 from SETTING::src/core/CompUnit/Repository/FileSystem.pm6:128  (/home/alex/.rakudobrew/moar-master/install/share/perl6/runtime/CORE.setting.moarvm:need)
 from src/Perl6/World.nqp:1274  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/World.moarvm:load_module)
 from src/Perl6/World.nqp:1202  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/World.moarvm:do_pragma_or_load_module)
 from gen/moar/Perl6-Grammar.nqp:1741  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:statement_control:sym<use>)
 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:statement_control)
 from gen/moar/Perl6-Grammar.nqp:1389  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:statement)
 from gen/moar/Perl6-Grammar.nqp:1317  (/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:1764  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:FOREIGN_LANG)
 from gen/moar/Perl6-Grammar.nqp:1281  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/Perl6/Grammar.moarvm:comp_unit)
 from gen/moar/Perl6-Grammar.nqp:552  (/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:1912  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:parse)
 from gen/moar/stage2/NQPHLL.nqp:1828  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:execute_stage)
 from gen/moar/stage2/NQPHLL.nqp:1861  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:run)
 from gen/moar/stage2/NQPHLL.nqp:1864  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:)
 from gen/moar/stage2/NQPHLL.nqp:1850  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:compile)
 from gen/moar/stage2/NQPHLL.nqp:1548  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:eval)
 from gen/moar/stage2/NQPHLL.nqp:1805  (/home/alex/.rakudobrew/moar-master/install/share/nqp/lib/NQPHLL.moarvm:evalfiles)
 from gen/moar/stage2/NQPHLL.nqp:1728  (/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:1654  (/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>)
@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 12, 2018

Member

Fails with something like this:

Tried to get the result of a broken Promise
  in method parallel_run at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 386
  in method parallel_parse at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 425
  in method parse_str at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 496
  in method large_parse at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 534
  in block <unit> at t/basic.t line 70

Original exception:
    Cannot invoke this object (REPR: Null; VMNull)
      in block  at /home/alex/git/MinG/lib/MinG/S13/Logic.pm6 (MinG::S13::Logic) line 18
      in method children_with_property at /home/alex/git/MinG/lib/MinG.pm6 (MinG) line 160
      in method exps at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 213
      in block  at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 382
Member

AlexDaniel commented Jul 12, 2018

Fails with something like this:

Tried to get the result of a broken Promise
  in method parallel_run at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 386
  in method parallel_parse at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 425
  in method parse_str at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 496
  in method large_parse at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 534
  in block <unit> at t/basic.t line 70

Original exception:
    Cannot invoke this object (REPR: Null; VMNull)
      in block  at /home/alex/git/MinG/lib/MinG/S13/Logic.pm6 (MinG::S13::Logic) line 18
      in method children_with_property at /home/alex/git/MinG/lib/MinG.pm6 (MinG) line 160
      in method exps at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 213
      in block  at /home/alex/git/MinG/lib/MinG/S13.pm6 (MinG::S13) line 382
@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 13, 2018

Member
Member

AlexDaniel commented Jul 13, 2018

jnthn added a commit to MoarVM/MoarVM that referenced this issue Jul 16, 2018

Use larger integer time to iterate strings heap
It was overflowing and resulting in the compilation failure reported
in rakudo/rakudo#2056. However, it's also
perhaps a little concerning that the file has a string heap of
275,305 strings.
@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

The fix in MoarVM/MoarVM@b7c6afd deals with the "MVMHash representation requires MVMString keys" error. The behavior change that led to so many strings is that instead of doing some C cheating, we now nqp::bindattr into Scalar, and that makes it eligible for serialization context repossession. That is the mechanism by which augment works.

Given the different failure modes of the other two tests, I don't think that the MoarVM fix I just did will address them. It is, however, possible that they have changed behavior for the same underlying reason.

Member

jnthn commented Jul 16, 2018

The fix in MoarVM/MoarVM@b7c6afd deals with the "MVMHash representation requires MVMString keys" error. The behavior change that led to so many strings is that instead of doing some C cheating, we now nqp::bindattr into Scalar, and that makes it eligible for serialization context repossession. That is the mechanism by which augment works.

Given the different failure modes of the other two tests, I don't think that the MoarVM fix I just did will address them. It is, however, possible that they have changed behavior for the same underlying reason.

@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

I put in a hack to never do the repossession on a Scalar and that didn't fix either of the other two. So the good new is that it's not due to that problem. The bad news is that it must therefore we due to some other as yet unknown problem.

Member

jnthn commented Jul 16, 2018

I put in a hack to never do the repossession on a Scalar and that didn't fix either of the other two. So the good new is that it's not due to that problem. The bad news is that it must therefore we due to some other as yet unknown problem.

@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

Ah, correction. The Scalar repossession issue is the cause of the MinG regression, but not the Spit one.

Member

jnthn commented Jul 16, 2018

Ah, correction. The Scalar repossession issue is the cause of the MinG regression, but not the Spit one.

jnthn added a commit that referenced this issue Jul 16, 2018

Before composition, typecheck against added roles
Previously:

    my $type = Metamodel::ClassHOW.new_type();
    $type.^add_role(Positional);
    say $type ~~ Positional;

Would produce `False`, because we didn't put together the set of roles
to type check against until class composition time. While it's true
that we cannot resolve transitive roles and parameterizations until
that time, it's reasonable enough to answer "yes" to the roles we have
been explicitly told about before composition time.

This came up because of a regression in Spit as reported in #2056. The
previous Scalar assignment code would in some cases not enforce type
checks properly in the presence of uncomposed types. When we switched
away from the pile of C code that repeated (sort of) the type checking
logic and towards using nqp::istype, we lost this bug. However, the
module relied on it. Thankfully, we can avoid breaking the module by
applying this quite reasonable change to Rakudo.
@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

The Spit one is fixed by 38d046f. A case where the new Scalar assignment code lacks a bug the previous one had, but thankfully there's a way to make fix a second bug and get it happy and passing all its tests again. So that's 2 out of 3.

Member

jnthn commented Jul 16, 2018

The Spit one is fixed by 38d046f. A case where the new Scalar assignment code lacks a bug the previous one had, but thankfully there's a way to make fix a second bug and get it happy and passing all its tests again. So that's 2 out of 3.

jnthn added a commit to perl6/nqp that referenced this issue Jul 16, 2018

Disable the SC write barrier during load
The load time of a module is dynamically within the scope of the
compilation of the using module. However, from a user perspective the
module mainline is probably more fittingly seen as a bit of runtime
during compile time.

So, we'll try it like this. The immediate motivation is the MinG issue
in rakudo/rakudo#2056. Its `our`-scoped
variables that are exported never used to have their assignments made
subject to precompilation repossession since Scalar's assignment didn't
use the SC write barrier. Now, Scalar assignment uses `bindattr` and so
is subject to the SC write barriering. That would count as a potential
bug fix in the case of meta-objects implemented in Perl 6 being used
with `augment` (not that any known cases of this exist). However, it
has the unfortunate consequence that assignments made in a module's
mainline now trip the barrier and end up serialized into the using
module's serialization context when a variable was exported.

This change together with the recent Rakudo change to make Scalar less
of a special case have the net effect of preserving existing behavior
of exported variables. It probably also makes the actual behavior of
code in a module's mainline a bit more runtime-y, and thus may well
help to make precompilation a little more transparent than it is today
also. Hopefully, with no downsides.
@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

NQP commit perl6/nqp@1ca664a deals with the MinG regression.

Verification of all of these would be very welcome.

Member

jnthn commented Jul 16, 2018

NQP commit perl6/nqp@1ca664a deals with the MinG regression.

Verification of all of these would be very welcome.

@jnthn jnthn added the testneeded label Jul 17, 2018

@jnthn jnthn removed their assignment Jul 17, 2018

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 18, 2018

Member

Looks good here!

Member

AlexDaniel commented Jul 18, 2018

Looks good here!

jnthn added a commit that referenced this issue Aug 14, 2018

Mark Scalar as never being repossessed
This restores a somewhat dubious, but relied upon, pre-rescalar
behavior. A previous attempt at fixing #2056 led to #2126. Together
with the revert in NQP commit 9fe83eb3e9, the modules in both tickets
now all work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment