Raise Invalid_arg if Hashtbl.create is given a negative init size

[MisterDA]

Fix #13469. This is akin to e.g., List.init.

[MisterDA]

cc @nikolaushuber @jmid

[jmid]

The fix LGTM 👍

Strictly speaking this is a backward-incompatible change.
We might want to discuss the issue before moving to a fix.

• It is indeed minor, so it might just fall under "won't fix" but "reasonable to document as an issue"
• If we move forward with the PR, we should be sure not to break too much code
  • Hashtbl.create is so pervasive! https://sherlocode.com/?q=Hashtbl.create%200 has ~779 results alone
  • From the skimming I've done I've seen no negative literals passed - opam repo tests would increase confidence though

Finally, as a Stdlib change this will require 2 core dev approvals.

[Octachron]

I like the fix in principle. Nevertheless, I worry that this fix can break perfectly working code whereas I struggle to imagine a situation where this fix would help identify a meaningful bug.

[avsm]

This seems like a regression to me; it'll potentially raise exceptions in currently working code, but add no real improvements. Wouldn't it be better to document the minimum and maximum values, and add a note that values outside this range will be clamped to those?

[gasche]

We discussed this at our triaging meeting today, and we couldn't really find overwhelming support in favor of the PR -- we agree that programs should not do this, but we don't necessarily agree that breaking them is the right reaction. (Of course if this was like this from scratch it would be fine.)

[OlivierNicole]

The majority opinion among maintainers is that the behaviour should not be changed, but the documentation should be.