fix: handle utf8 characters in the dune files(#9728)

[moyodiallo]

This PR is about principally handling utf8 characters in quoted strings.

[rgrinberg]

Why do we need to copy paste code from the stdlib?

[moyodiallo]

Why do we need to copy paste code from the stdlib?

No need, we could write our own functions. But I did this because I used functions introduced in OCaml.4.14 (later version).

[rgrinberg]

The issue isn’t the copy pasting of some code itself, it’s just that I think we’re copying too much. Don’t we already have a subset of Uchar present since our minimum version is 4.08? Could we just add only the code we need?

…

On Feb 22, 2024, at 11:15 AM, Alpha Issiaga DIALLO ***@***.***> wrote: Why do we need to copy paste code from the stdlib? No need, we could write our own functions. But I did this because I used functions introduced in OCaml.4.14 (later version). — Reply to this email directly, view it on GitHub <#10113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABB56Z5TEQ43NYGCIMYW3DYU6KOJAVCNFSM6AAAAABDU5EZVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRQGA4TEMBVGE>. You are receiving this because you commented.

[moyodiallo]

I've just tested uutf, I'm going to use vendor/uutf present in Dune project. That is sufficient for this PR.

[moyodiallo]

Indeed uutf use Uchar which is present in OCaml 4.08.

[moyodiallo]

The copy paste is removed and uutf is used instead.

[rgrinberg]

It's a bit sad, but this PR has a non trivial increase in pressure on the GC. Since this fix only affects a tiny fraction of users, is it possible to implement this in a less intrusive way?

[moyodiallo]

Yes it is possible. I'll try to fix it.

[emillon]

I don't think that the lexer changes are useful for this change. Just changing Escape will fix the formatter, which was the original issue. The parsing error for %{...} will continue refering to individual bytes, but I think that's fine compared to changing the lexer.

[moyodiallo]

I don't think that the lexer changes are useful for this change. Just changing Escape will fix the formatter, which was the original issue. The parsing error for %{...} will continue refering to individual bytes, but I think that's fine compared to changing the lexer.

I get it but it won't be coherent, accepting utf8 char and printing escaped characters.

[moyodiallo]

It's a bit sad, but this PR has a non trivial increase in pressure on the GC. Since this fix only affects a tiny fraction of users, is it possible to implement this in a less intrusive way?

It's fixed.

[emillon]

this will also require a changelog entry

[moyodiallo]

this will also require a changelog entry

The changelog entry added.

[emillon]

Thanks. I'll adapt the API so that it's more in line with the stdlib API and thus easier to remove later.

[emillon]

I tried to use the stdlib way, but doing so and adding polyfills for 4.08 meant that stdune would depend on uutf. So instead I adapted the code to use uutf decoder (that work on 4.08) in dune_sexp with a note on how to migrate to stdlib when we bump the compat.

[emillon]

@rgrinberg is the library structure (dune_sexp depending on dune_uutf) sensible?

[rgrinberg]

Seems fine to me. Make sure to double check the benchmarks before merging.

[emillon]

Benchmarks look good so I'm merging this, thanks.