Add OCaml support #426

Merged

wetneb merged 1 commit from thufschmitt/mergiraf:ocaml-support into main

2025-06-03 23:42:56 +02:00

thufschmitt commented

2025-06-03 16:22:46 +02:00

Contributor

This adds effectively two different languages, because there are two different tree-sitter syntaxes for the OCaml implementation and interface files.

Only a very simple case of commutative parents is provided, there might be more to add in the future (though it's unclear because very few things are fully unordered in OCaml).

This adds effectively two different languages, because there are two different tree-sitter syntaxes for the OCaml implementation and interface files. Only a very simple case of commutative parents is provided, there might be more to add in the future (though it's unclear because very few things are fully unordered in OCaml).

thufschmitt added 1 commit

2025-06-03 16:22:47 +02:00

Add OCaml support

/ test (pull_request) Successful in 1m43s

Details

9a9b1a7e82

wetneb commented

2025-06-03 16:38:55 +02:00

Owner

Thanks a lot for this! Yay for OCaml, it reminds me of the good old days when I learned OCaml Light at school :)

The only slight concern I have is that this grammar is known to have a really big parsing table. Adding it to Mergiraf makes the release binary size on x86_64 arch jump from 47MB to 57MB. I don't know if OCaml's adoption justifies adding 10MB to the binary for every user.

As long as we won't support dynamic language loading (#357), I guess it will be hard to make a judgment call on this sort of issue. We've been integrating parsers for languages that are arguably more niche than OCaml, of course…

Maybe we can merge this PR as is for now, with the warning that once we have dynamic language loading, this language will be a likely candidate for eviction from the standard binary, migrating it to a dynamically loaded one. @ada4a, what do you think?

Thanks a lot for this! Yay for OCaml, it reminds me of the good old days when I learned OCaml Light at school :) The only slight concern I have is that this grammar is known to have a really big parsing table. Adding it to Mergiraf makes the release binary size on x86_64 arch jump from 47MB to 57MB. I don't know if OCaml's adoption justifies adding 10MB to the binary for every user. As long as we won't support dynamic language loading (#357), I guess it will be hard to make a judgment call on this sort of issue. We've been integrating parsers for languages that are arguably more niche than OCaml, of course… Maybe we can merge this PR as is for now, with the warning that once we have dynamic language loading, this language will be a likely candidate for eviction from the standard binary, migrating it to a dynamically loaded one. @ada4a, what do you think?

thufschmitt commented

2025-06-03 17:49:10 +02:00

Author

Contributor

Thanks for the very quick reply @wetneb !

Fwiw, I'm obviously in favour of adding OCaml to Mergiraf ;)

If that's not too hard to implement (I don't know the codebase I just blindly followed the – great – tutorial on adding a language), I'm also happy to hide the OCaml support behind a cargo feature flag so that it's not included by default

Thanks for the very quick reply @wetneb ! Fwiw, I'm obviously in favour of adding OCaml to Mergiraf ;) If that's not too hard to implement (I don't know the codebase I just blindly followed the – great – tutorial on adding a language), I'm also happy to hide the OCaml support behind a cargo feature flag so that it's not included by default

ada4a commented

2025-06-03 17:55:52 +02:00

Owner

I don't know if OCaml's adoption justifies adding 10MB to the binary for every user.

Honestly, I'm pretty indifferent to the whole binary size problem – I don't really know what size should be considered too big... With that in mind, I'd say that, while we're a relatively small project (I think?), having at least one person go through the whole effort of opening a language-addition PR is a good indicator of a big enough interest in having that language, so it makes sense to have it.

Maybe we can merge this PR as is for now, with the warning that once we have dynamic language loading, this language will be a likely candidate for eviction from the standard binary, migrating it to a dynamically loaded one.

You mean emitting a warning every time a merge is performed with OCaml? I imagine that would get pretty annoying pretty fast. Or do you mean just mentioning it in the release notes? That would make sense to me, yes

> I don't know if OCaml's adoption justifies adding 10MB to the binary for every user. Honestly, I'm pretty indifferent to the whole binary size problem – I don't really know what size should be considered too big... With that in mind, I'd say that, while we're a relatively small project (I think?), having at least one person go through the whole effort of opening a language-addition PR is a good indicator of a big enough interest in having that language, so it makes sense to have it. > Maybe we can merge this PR as is for now, with the warning that once we have dynamic language loading, this language will be a likely candidate for eviction from the standard binary, migrating it to a dynamically loaded one. You mean emitting a warning _every time_ a merge is performed with OCaml? I imagine that would get pretty annoying pretty fast. Or do you mean just mentioning it in the release notes? That would make sense to me, yes

👍 1

wetneb approved these changes

2025-06-03 18:13:47 +02:00

wetneb left a comment

Owner

I only meant "warning" in the sense of "warning @thufschmitt in this PR that we might not keep OCaml support built-in forever". Indeed the mechanism to warn upgrading users remains to be figured out…