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

Add to packfile API and remove Eval PMC #937

Closed
wants to merge 4 commits into from

Conversation

gerdr
Copy link
Contributor

@gerdr gerdr commented Feb 22, 2013

This is a cleaned-up version of #934. Needs to be applied together with the corresponding changes to NQP and Rakudo.

Additionally, add method first_sub_in_const_table() to PackfileView PMC as
a stopgap measure until properly tagged subs are generated.
@gerdr gerdr mentioned this pull request Feb 22, 2013
@leto
Copy link
Member

leto commented Mar 25, 2013

What is the status of this?

@gerdr
Copy link
Contributor Author

gerdr commented Mar 25, 2013

Status: Dormant, hopefully not dead.

It's a two-way breaking change and pmichaud holds the patch pumpkin. The NQP side of things has bitrotten after jnthn did some refactoring in HLL::Compiler.

@pmichaud
Copy link
Member

On Mon, Mar 25, 2013 at 10:30:01AM -0700, Gerhard R. wrote:

Status: Dormant, not dead.

It's a two-way breaking change and pmichaud holds the patch
pumpkin. The NQP side of things has bitrotten after jnthn
did some refactoring in HLL::Compiler.

Sorry, didn't realize this change was waiting on me.
The Parrot side of the patch can go in anytime after the 5.3.0
release, we'll update the NQP and Rakudo sides to match shortly
after that.

Or if there's an up-to-date Parrot branch with the changes
enabled, we'll create a nqp and rakudo branches to
match/test the changes and then both teams can merge their
branches when everything is ready.

Pm

@Benabik
Copy link
Member

Benabik commented Oct 8, 2013

This PR has been merged with master and pushed as the new-packfile-api branch.

Note: I did not test this branch but I wanted to make it available for NQP & Rakudo to test.

rurban pushed a commit that referenced this pull request Oct 8, 2013
ending whitespace in t/pmc/sub.t
superfluous get_packfile_eval_pmc in src/pmc/imccompiler.pmc
pmichaud added a commit that referenced this pull request Oct 9, 2013
This short patch adds two methods to EvalPMC (deprecated)
that provides a forward-compatible API for the eventual
PackfileView PMC conversion.  NQP/Rakudo can then switch
to the new API even before PackfileView lands in master,
which greatly aids NQP's bootstrapping process.  See issue #937.
@pmichaud
Copy link
Member

pmichaud commented Oct 9, 2013

Tonight I created a "pm-packfile-api" branch in the nqp repo that is able to work with the new-packfile-api branch in Parrot. The pm-packfile-api nqp branch is also able to work with Parrot's master branch (using EvalPMC) after commit 07dfdb4. We'll likely merge the pm-packfile-api branch in the next few days, or shortly after Parrot's October release.

So, Parrot should be able to switch over to PackfileView at any time without it negatively impacting nqp or rakudo.

Pm

@leto
Copy link
Member

leto commented Oct 9, 2013

Great to see this activity! Just for reference, are we keeping this issue open until pm-packfile-api branch gets merged, or can it be closed now?

@pmichaud
Copy link
Member

pmichaud commented Oct 9, 2013

I recommend keeping the issue open until the new-packfile-api branch is merged.

(The "pm-packfile-api" branch is a nqp branch; it's related to this issue, but shouldn't be considered a blocker for it.)

Pm

@pmichaud
Copy link
Member

The pm-packfile-api branch in NQP has been merged to master, so a Parrot switch to PackfileView should be relatively seamless.

Pm

rurban pushed a commit that referenced this pull request Oct 26, 2013
evalpmc can now be removed.
rurban pushed a commit that referenced this pull request Oct 26, 2013
@rurban rurban closed this Mar 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants