fix: move the UTF8 transform to the compiler core

[anmonteiro]

• found in ppx: show strange quoted strings transform #863
• moves the utf8 transformations from the ppx to the compiler core, to make sure they're applied last
• still missing the integration with [@@mel.inline], as can be seen in the snapshotted test case

[jchavarri]

I am not sure i fully understand what the utf8 string processing does, but I have a couple of questions:

• Does this PR remove the need of ppx authors to process j* attrs? If yes, I understand it'll be done in another PR? couldn't find any refs to that in this one.
• Should Melange be able to process utf strings like \u{1F680}? I tried on the playground but the ppx will error out.
• Do you think it'd make sense to test the UTF8 end to end with the ppx? Right now I can see some direct tests of Utf8_string.Interp.transform_test but I can't find anything that uses the j or js attrs.
• Finally, what are the downsides of this approach vs the options we discussed in the original issue?

Thanks!

[jchavarri]

Why does the transform have to be moved from the expression to the structure item section?

[anmonteiro]

That's not what's happening here. This code concerns only [@@mel.inline], which transforms let x = {j| ... |j} [@@mel.inline] into an external with Ffi_inline_const (Const_string {s;unicode}).

We perform this transformation in the PPX because there's code that can be inlined, and code that can't. Example:

let x = {j| foo |j} [@@mel.inline] (* <-- constant string, can be inlined *)

(* will be transformed into `"foo" ^ x`, which isn't a constant, and cannot be inlined *)
let y = {j| foo $(x) |j} [@@mel.inline] 

[jchavarri]

That's not what's happening here.

I am confused. This code seems to have been moved from line 405, where it was inside the method! expression e section, into this section, which is the method! structure_item str one.

We perform this transformation in the PPX because there's code that can be inlined, and code that can't.

Makes sense.

[anmonteiro]

It’s moved to structure item to catch the only the inlined cases. The remaining transformations happen in the core lib now

[anmonteiro]

Does this PR remove the need of ppx authors to process j* attrs? If yes, I understand it'll be done in another PR? couldn't find any refs to that in this one.

Yes, it does. By moving the transformation to the compiler, there are no longer cases where *j appears in the AST that PPX authors interact with.

Should Melange be able to process utf strings like \u{1F680}? I tried on the playground but the ppx will error out.

This is not a Melange issue. It works if you use OCaml syntax, and it's not a feature of {j| ... |j}. Js.log "\u{1F680}" will do what you expect.

For Reason syntax, you're running into reasonml/reason#2337

Do you think it'd make sense to test the UTF8 end to end with the ppx? Right now I can see some direct tests of Utf8_string.Interp.transform_test but I can't find anything that uses the j or js attrs.

These are tested in the runtime tests under jscomp/test. The fact that we don't see a diff in the snapshotted JS means that they're working end-to-end. In fact, you'll see that the first commit (3e6f7d6) broke {j| .. |j} + [@@mel.inline] and produced such a diff.

Finally, what are the downsides of this approach vs the options we discussed in the original issue?

The downsides here are:

• we call Utf8_string.Interp.transform in 2 places rather than 1:
  1. in the compiler core, for processing regular strings
  2. in the PPX, to transform [@@mel.inline]d strings into externals
• the unicode string transformation is sort of a "compiler frontend" job that's being done in the core compiler (which is a bit against [melange.ppx]: move the entire PPX frontend to melange.ppx #583), but needs to run after all the external PPXes have been applied.

Other than that, I think this actually solves the problem quite cleanly and has the additional benefit of merging the utf8 string code back into one module in jscomp/common.

[jchavarri]

Do you think this will have impact on compilation times? Is there a way to measure it?

[anmonteiro]

I don’t think so. What we were doing in the ppx before, we’re doing here now. The additional transformation is happening for Mel.inline which is a very small subset of cases