stdlib: add Array.init_matrix

[gmevel]

This PR adds Array.init_matrix to the standard library, with the obvious semantics. It fills a gap because:

1. a user expects to find this function, considering that there exist Array.make and Array.init and Array.make_matrix;
2. just like Array.make_matrix, it reduces the risk of someone falling into the beginner’s trap of initializing an array with pointers to a shared mutable value.

I included a test.

Remaining questions:

1. I would also add this function to Float.Array, but apparently there is no Float.Array.make_matrix. Is this intentional?
2. The current docstring does not specify in which order the various invokations of f take place; should it? (I incline to think it should not, but this contrasts with the current docstring of Array.init.)
3. This is my first PR. I am not sure what is the correct syntax for this docstring: @since NEXT_OCAML_RELEASE.

[xavierleroy]

I think it's a good addition to the Array module. I'm not 100% happy with the documentation, but have nothing better to suggest at this point.

[gasche]

I agree that the function is reasonable and the implementation is fine.

[gasche]

cc @nojb, who can tell us whether we should indeed add Floatarray.{make,init}_matrix.

[yallop]

I'm in favour of adding this, but I'd prefer an implementation that went all in on either efficiency or elegance, e.g.

let init_matrix sx sy f =
  init sx @@ fun x ->
  init sy @@ fun y ->
  f x y

(or perhaps one day let init_matrix3 sx sy f = [| [|f x y for y = 0 to sy-1|] for x = 0 to sx-1 |])

Also, a documentation nit:

 @raise Invalid_argument if [dimx] or [dimy] is negative or

If dimx is 0 and dimy is negative, it doesn't raise.

[gasche]

I also found the implementation a bit odd, until I realized that it is adapted from make_matrix above which is structured in the same way.

[gmevel]

Indeed I would have written the elegant version but I figured that maybe make_matrix was written that way for performance reasons, so I mimicked its code. I guess what @yallop is suggesting is that, if opting for ugly/efficient code, I should also inline the call to init as well and thus avoid the closure? I just pushed that change.

Regarding the doc nitpick (that part was copy-pasted from make_matrix): good catch, however I’m not sure whether we want to specify that dimx = 0 && dimy < 0 is allowed. It means a more complex sentence in doc, I’m not sure it has practical benefits, and it could be regarded an accident of the implementation. Case in point, my reworking of the implementation has changed the behavior in this corner case (it now checks dimy < 0 before create-ing the outer array; that’s not necessary, we could also swap these operations).

[nojb]

cc @nojb, who can tell us whether we should indeed add Floatarray.{make,init}_matrix.

I looked in LexiFi's codebase, there are a handful of occurrences of this function. So I would be in favour of adding them.

[gmevel]

I thus added Float.Array.{make,init}_matrix.

Regarding the edge case dimx = 0 && dimy < 0 found by @yallop: while addressing a reviewing comment by @nojb, I aligned the behavior of make_matrix with that of init_matrix: now, both functions raise Invalid_argument (as do the new functions in Float.Array). I added a breaking change mention in the changelog. It has been the documented specification since 1999 (commit 10270af).

[gasche]

I approved this already. If we want to accept this PR, we need a second approver -- because it is stdlib. In the past I have felt uneasy about being invited to second-approve because it is hard to say "no". So now I am hesitating to name names.

[nojb]

LGTM

[gasche]

The CI has a failure of memory-model/forbidden.ml, which is unrelated.