search

NathanReb / ppx_yojson

OCaml PPX extension for JSON literals and patterns
BSD 2-Clause "Simplified" License
42 stars 5 forks source link

Remove unnecessary metaquot extensions #26

Closed NathanReb closed 2 years ago

NathanReb commented 3 years ago

ppx_yojson makes quite an heavy use of metaquot as it simplifies the code quite a lot.

That's possible thanks to metaquot "anti-quotation" which allows injecting ast fragments into a metaquot payload.

When browsing through the source recently I stumbled to a few occurences of useless metaquot extensions where the payload simply was an antiquoted expression building the appropriate ast fragment, i.e. something of the form:

[%expr [%e Ast_builder.Default.estring ~loc some_string]]

which can be simplified to

Ast_builder.Default.estring ~loc some_string

It would be nice to get rid of those.

ayc9 commented 3 years ago

Looking at the metaquot readme, it seems out of the 5 antiquotation expressions only [%e and [%p are used in ppx_yojson.

Searching for those + Ast_builder, you find some uses where there is a type before the antiquotation, like `String in:

[%expr `String [%e Ast_builder.Default.estring ~loc s]]

Does that matter? Since it's still explicitly building an ast, should those also be simplified? What is the purpose of specifying `String in such a case?

ayc9 commented 3 years ago

I'm an Outreachy applicant for this Oct 2021 contribution period. I started working on this issue and a first try pr is found here. I hope this is a good first step towards a contribution!

NathanReb commented 3 years ago

Sorry for the delay! I tried answering your question on the PR directly.

ayc9 commented 2 years ago

Hi @NathanReb This issue can be closed probably!