Add configuration to control whether let-punning is used

[WardBrian]

Adds option --let-binding-punning={preserve|always|never} to control whether ocamlformat outputs
let punning (open to suggestions on naming).

Default is preserve in all profiles to match the current release's behavior.

Closes #2743

[EmileTrotignon]

I think this is good, except maybe the documentation which could have an exemple and more explicit wording.

[EmileTrotignon]

I know this is not the style of the current documentation, but I think an example of what let punning is should included.

Also regarding the name, I thought a let-binding is any use of let, but this only concerns bindings with let operators ? This does not have to be reflected in the name, but the documentation should probably say something.

[Julow]

The problem is that it's not possible to add line breaks in the documentation, forcing the explanation to be concise. This is the output of cmdliner, which removes line breaks and reflow the text.

I'd be totally in favor of a new way of generating the list of option in the documentation and taking advantage of Odoc markup (paragraph, lists, text blocks). The cmdliner output must stay thought.

[WardBrian]

I've changed the name to your suggestion from the issue, letop-punning. I also made an attempt at including an example in this doc, let me know what you think!

[Julow]

The doc looks good to me ! It's a bit messed up because of the way we pass it through cmdliner but that can be improved independently.

[EmileTrotignon]

The problem is that it's not possible to add line breaks in the documentation, forcing the explanation to be concise. This is the output of cmdliner, which removes line breaks and reflow the text.
I wasn't aware, thanks. I agree it would be better with improvement

[Julow]

Other than the discussion on documentation, I'm totally in favor of this change :)

[Julow]

The problem is that it's not possible to add line breaks in the documentation, forcing the explanation to be concise. This is the output of cmdliner, which removes line breaks and reflow the text.

I'd be totally in favor of a new way of generating the list of option in the documentation and taking advantage of Odoc markup (paragraph, lists, text blocks). The cmdliner output must stay thought.

[Julow]

To make the difference between never and preserve clearer, I think you should say that existing code using let-punning will be rewritten without.

[WardBrian]

Question:
I have the code prepared to also have this setting work for extension lets (e.g. let%bar x = x in ... becomes let%bar x in ....

Would this be better in this PR or in another PR? I don't currently have 'preserve' working for these, because that requires parser changes, but I think I could get that done

[Julow]

I have the code prepared to also have this setting work for extension lets (e.g. let%bar x = x in ... becomes let%bar x in ....

That's fantastic !
I usually prefer several smaller PRs instead of a large one. Now that this one is reviewed, let's merge it :)

[EmileTrotignon]

Would this be better in this PR or in another PR? I don't currently have 'preserve' working for these, because that requires parser changes, but I think I could get that done

This is the type of parser changes that we try to do, so please do that ! Don't forget to have the loc of every node you add (maybe in that case all the locs are here already)

Thats because comments moving around are caused by not knowing the loc of certain tokens, and therefore having no way of knowing if the comment was before or after said token.

[EmileTrotignon]

I am merging once the CI is green (except for that benchmark which always fails)

[WardBrian]

Thanks @EmileTrotignon, I was wondering about that one. The rest all look like they passed

[EmileTrotignon]

Yeah, I think the benchmark has a dependency on libpcre3dev which is a recently defunct debian package.

Lets merge !