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

Fix for a rare case of empty mast_frames hash #569

Closed
wants to merge 1 commit into from

Conversation

vrurg
Copy link
Contributor

@vrurg vrurg commented Aug 5, 2019

Caused by a bug reported in rakudo/rakudo#3096. This fix is somewhat of
a guess because I'm not familiar with QAST->MAST compilation. But the
logic I put behind it: if no frames found in mast_frames then just do
nothing.

Caused by a bug reported in rakudo/rakudo#3096. This fix is somewhat of
a guess because I'm not familiar with QAST->MAST compilation. But the
logic I put behind it: if no frames found in mast_frames then just do
nothing.
vrurg added a commit to vrurg/rakudo that referenced this pull request Aug 5, 2019
Intended to fix rakudo#3096 but wouldn't work without
Raku/nqp#569.

The point is that a lot of code in World.nqp expects %?OPTIONS key on
%*COMPILING but CompUnit::Loader didn't set it.
@vrurg vrurg requested a review from niner August 5, 2019 01:13
@vrurg vrurg added the Moar label Aug 5, 2019
@vrurg vrurg requested a review from jnthn August 5, 2019 01:13
Copy link
Contributor

@niner niner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think this is the right approach. It would simply skip adding those static code refs to the serialization context. It may hide some immediate errors, but cause more obscure ones later on.

Note that compile time eval is only somewhat implemented. There are still missing bits and pieces and hacks in place of real implementations, so a quick and easy fix may just not do in this case.

@vrurg
Copy link
Contributor Author

vrurg commented Aug 5, 2019

I do not think this is the right approach. It would simply skip adding those static code refs to the serialization context. It may hide some immediate errors, but cause more obscure ones later on.

That's why I requested for a review. I felt like it's not something correct even though it made the EVAL work. Still, I wonder if there should be a rather simple fix. Perhaps, in ForeignCode.pm6. Considering line https://github.com/rakudo/rakudo/blob/master/src/core/ForeignCode.pm6#L54 and https://github.com/rakudo/rakudo/blob/master/src/core/ForeignCode.pm6#L74 I guess that the empty hash somehow makes it all they way down to the list_b code. I guess a frame must be added to that hash then. But which one? $compiled?

Note that compile time eval is only somewhat implemented. There are still missing bits and pieces and hacks in place of real implementations, so a quick and easy fix may just not do in this case.

Noted. I'm closing this PR as wrongful.

@vrurg vrurg closed this Aug 5, 2019
vrurg added a commit to rakudo/rakudo that referenced this pull request Aug 5, 2019
Fix for unset `%?OPTIONS` key on `%*COMPILING` which was making `World` code unhappy.

This is a step to get #3096 fixed. As discussed in Raku/nqp#569, more fixes needed to make `EVAL` work at compile time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants