IH-633: Transition away from exception-raising Hashtbl.find and Unix.getenv

[last-genius]

This PR starts the transition away from exception-raising OCaml's stdlib functions Unix.getenv and Hashtbl.find in favour of Sys.getenv_opt and Hashtbl.find_opt respectively.

This is beneficial because:

• It handles failure explicitly (in a less expensive way than with exceptions, if failure was handled before in try-catch blocks), as an element of the interface, not a source code/documentation issue.
• It avoids two traversals where .find functions where gated by .mem functions to avoid raising exceptions, instead doing just one.

A naive benchmark shows the benefit:

┌───────────────────────────────────────────────┬──────────┐
│ Name                                          │ Time/Run │
├───────────────────────────────────────────────┼──────────┤
│ id                                            │   0.93ns │
│ Option.value (Hashtbl.find_opt) ~default      │  13.58ns │
│ Hashtbl.mem then find                         │  27.05ns │
└───────────────────────────────────────────────┴──────────┘

• It avoids a potential data race between checking if an element is present (.mem) and getting its value (.find)
• It avoids a potential human error where .mem would be changed but its corresponding .find would not be.

===========

This PR is structured in the following way:

• There is one commit for the Unix.getenv -> Sys.getenv_opt change along with the quality gate test for it. Unix.getenv usage has been completely eliminated.
• There is one commit for trivial cases of Hashtbl.find -> find_opt
• There are two commits for more complex refactoring for Hashtbl.find -> find_opt, in one case I've eliminated unnecessary Hashtbl operations altogether.
• There is one commit for the quality gate test for Hashtbl.find - there are 36 usages left that do not handle exceptions currently and would need to introduce it on the appropriate level.

Even though the PR is relatively large, most changes are trivial and fall into several classes:

• Exception already handled
  • Simple try-catch to provide default value => transition to Option.value
  • Some logic in try-catch => transition to match statement instead
  • Re-implementation of _opt by doing try Some (x.find) with _ -> None => just x.find_opt
• .find gated by .mem => transition to match statement / Option.iter / Option.map depending on the case

===========

These changes passed the BST+BVT test suites.

[last-genius]

Due to the size of this PR, I will be pushing fixup commits to ease the review process. Will squash these at the end.

[last-genius]

~Pedantically, there is a case where you can η-reduce (fun s -> Stunnel.disconnect s).~ - nevermind, we shouldn't be making unrelated changes here (I guess do them if you want so long as they are obviously correct and won't impair review?).

Nice catch! I've been trying to keep this PR to changes that do not change the logic of code -- this one does not, however, a simple grep indicates there are at least 50 such cases in the codebase!

$ unbuffer rg 'fun\s+(\w+)\s+->\s+\w+\s+\1\)' --pcre2 --no-heading | tee >(wc -l)
[....]
50

Might just as well put this in a separate ticket/PR - though there is little practical benefit, it gets boiled down to the same code

Also, if you want to be consistent with using Option.map and friends to outline casing behaviour, there are a few cases where fold could be used, e.g.:

I myself find that the parseability of match is better than that of Option.fold -- perhaps it's just a question of style and preference. match is not so nice when one of the arms is heavy and the other is empty, so that's why I've used .map and .iter to immediately indicate the intention without having to search for the other arm.

[contificate]

Might just as well put this in a separate ticket/PR - though there is little practical benefit, it gets boiled down to the same code

Indeed, it's only a stylistic concern; this case is a minor one but there are areas of the codebase where eta-reduction could be applied several times, which cleans things up a lot, visually (typical examples would be with List.fold_left). It's one of the more pedantic style changes, but it's generally accepted as good practice (so long as you don't get carried away with point-free style and get caught out by the value restriction).

[last-genius]

Had to rebase on top of master to get the Clock module, so hashes have changed. Otherwise review fixes are still in separate fixup commits.

[coveralls]

coverage: 44.887%. remained the same when pulling dd2718061588df3e65f0a6fa69b8452e33f7b2d8 on last-genius:private/asultanov/opt-refactoring into ee8e80032f17c97e0f2aa4952a531b81a5c0ce94 on xapi-project:master.

[lindig]

Has your comment being addressed, @psafont? Please revisit this such that we can merge this.

[psafont]

Please squash the commits

[last-genius]

Squashed the fixes. Commits are still split into several as was suggested by Christian initially, so that we could revert only one if needed later (especially in the case of the more complex refactored files)