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

Might it make sense to upstream this to ppxlib.metaquot? #4

Open
pitag-ha opened this issue Nov 22, 2021 · 3 comments
Open

Might it make sense to upstream this to ppxlib.metaquot? #4

pitag-ha opened this issue Nov 22, 2021 · 3 comments

Comments

@pitag-ha
Copy link

If I'm not missing anything, this version of metaquot is strictly better than ppxlib.metaquot in the sense that its features are a super-set of the ppxlib.metaquot features. Is that right? Are there maybe also some downsides to it that I'm currently not seeing (performance or similar)?

I think it would be great to discuss the context around that a little! Concretely, why is this a separate project instead of being upstreamed to ppxlib.metaquot? Are there any technical issues that upstreaming it would involve? Are some of the features that are here and not in ppxlib.metaquot controversial?

Of course, we wouldn't upstream this from one day to another, but it might be worth looking into that, so I'd love to hear your opinion @thierry-martinez!

@thierry-martinez
Copy link
Owner

thierry-martinez commented Nov 23, 2021 via email

@pitag-ha
Copy link
Author

Thanks for the context and history on this, that's very interesting! I understand far better now why this metaquot co-exists with ppxlib.metaquot.

The most interesting thing I see in metaquot is that it is mainly automatically generated by preprocessing compiler-libs' Parsetree module with metapp

I see! Part of that might be similar to ppxlib.metaquot, which uses a "lift" class that's generated by preprocessing the Ppxlib AST: a class that for each node in the Ppxlib AST provides a method to capture that node representation in the given target: expression or pattern.
And about metapp: I think the reason why in this metaquot it comes in very useful (and in ppxlib.metaquot it's not needed) is that here you need to make the cross-compiler compatibility explicit (whereas in ppxlib.metaquot it's abstracted away by using the Ppxlib AST). Is that correct?

I would be glad if metapp itself becomes more visible in the ppx-ecosystem, because I think it could be very useful for the community

metapp does indeed seem very cool! It has a broader use case than ppx_optcomp and cppo, doesn't it? (although, of course, that's a different topic)

I am not sure that additional features of metaquot are worth being integrated to upstream ppxlib.metaquot

The features I was thinking about are [@subst] and [@for]. I've heard some requests about adding those two to ppxlib.metaquot. Have you tried them in some projects of yours and have noticed that they're less useful / more problematic than they seem?

@orbitz
Copy link

orbitz commented Jan 14, 2022

FWIW, [@subst] would be really great if it got upstream to pplib.metaquot somehow. I find that very useful, otherwise in ppxlib.metaquot I end up having to write out the parse tree by hand too often.

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

No branches or pull requests

3 participants