Add bigarray to the stdlib · Pull Request #1685 · ocaml/ocaml

ghost · 2018-03-30T00:19:05Z

This PR is the next step in moving Bigarray to the Standard Library.

Overview

It moves the Bigarray module to the standard library without the map_file functions. The bigarray library is kept for compatibility purposes and simply extends Stdlib.Bigarray with the deprecared map_file functions. This way users have more time to update their code, especially the ones who want to keep compatibility with OCaml < 4.06.0.

Other relevant details:

otherlibs/bigarray/bigarray_stubs was appended at the end of byterun/bigarray.c
the CamlinternalBigarray module was removed since it is no longer needed

Limitation

Because all the cmi files are installed in the same directory, in the initial environment the module name Bigarray always resolves to Bigarray from the bigarray library rather than Stdlib.Bigarray. One has to use Stdlib.Bigarray explicitely in order to use bigarrays without depending on unix.

Reviewing

Because some code was moved around, the diff shows a lot of added and deleted code. It is easier to review the following files with these commands:

stdlib/bigarray.ml: patdiff <(git show trunk:otherlibs/bigarray/bigarray.ml) stdlib/bigarray.ml
stdlib/bigarray.mli: patdiff <(git show trunk:otherlibs/bigarray/bigarray.mli) stdlib/bigarray.mli
byterun/bigarray.c: patdiff <(git show trunk:byterun/bigarray.c; git show trunk:otherlibs/bigarray/bigarray_stubs.c) byterun/bigarray.c

shindere · 2018-03-30T09:03:01Z

FWIW, ocamltest has a bigarray modifier to include the bigarray library under otherlibs. Am I correct that this PR would make the modifier un-necessary? If I amcorrect, then the PR could include its removal and perhaps also an update to the tests that makeuse of it.

ghost · 2018-03-31T00:37:26Z

Indeed, I removed the modifier

dra27 · 2018-04-02T13:55:25Z

~~Assuming that repeatably fixes it,~~ feel free to squash that commit into 97e616c

mshinwell · 2018-04-04T13:21:15Z

I am reviewing this.

mshinwell · 2018-04-09T11:05:17Z

This is going to produce an unrelated test failure which I have emailed @shindere about, but this patch is OK, and we should merge before 4.07. A bootstrap commit will be needed to remove some redundant primitives; I will do this.

…stdlib

mshinwell · 2018-04-09T12:13:54Z

The only failure for a reasonable set of targets appears to be the one which @shindere is currently working on, and I need to follow this with a manual bootstrap commit anyway, so I am going to merge this now.

mshinwell · 2018-04-09T12:33:04Z

The bootstrap which completes this GPR is in e866ba3f6891a18ede3235f125babd5f7fd6d03a.

dbuenzli · 2018-04-09T14:31:41Z

This closes MPR6139

gasche · 2018-04-09T14:38:03Z

Thanks @dbuenzli, I'll add the mantis issue to the Changes entry.

shindere · 2018-04-18T13:55:27Z

@diml sorry for asking this question so late.

Am I correct that, as far as the source tree is concerned, the bigarray
files were not moved to the stdlib directory?

May I ask why and whether there is any plan to do so in a further PR?

ghost · 2018-04-18T14:04:56Z

They were moved, however the bigarray library is kept for compatibility. The Bigarray.*.map_file functions were deprecated in 4.06, so we just need to wait a reasonable amount of time before deleting the bigarray library entirely. I assume that deleting it in 4.08 should be ok.

shindere · 2018-04-18T14:21:21Z

Jérémie Dimino (2018/04/18 14:05 +0000):

They were moved, however the bigarray library is kept for compatibility. The `Bigarray.*.map_file` functions were deprecated in 4.06, so we just need to wait a reasonable amount of time before deleting the bigarray library entirely. I assume that deleting it in 4.08 should be ok.

OK I must really have missed something, sorry. Which .cma file is one supposed to use when trying to use the bigarray lib from the sources (without installing?)

ghost · 2018-04-18T14:35:42Z

You should just use the Bigarray module from the stdlib, so you don't need any special cma. However, if you pass -I [...]/stdlib -I [...]/otherlibs/bigarray to the compiler, you need to use Stdlib.Bigarray explicitly in your code in order to refer to the bigarray module from the stdlib. After installing, you always need to use Stdlib.Bigarray. This is because the bigarray.cmi file in the search path has precedence over Stdlib.Bigarray. bigarray.cmi is from the bigarray library.

If you do want to use the deprecated bigarray library, then you need to to use bigarray.cma as before.

shindere · 2018-04-18T14:43:42Z

Jérémie Dimino (2018/04/18 14:35 +0000):

You should just use the `Bigarray` module from the stdlib, so you don't need any special cma.

I do need to handle the cmas directly, see for instance the lib-dynlink-csharp test, you can git grep for bigarray.cma in this directory and you'll see.

ghost · 2018-04-18T14:45:38Z

Ok, this test should still work as bigarray.cma still exists at the same place.

shindere · 2018-04-18T14:50:57Z

Jérémie Dimino (2018/04/18 14:45 +0000):

Ok, this test should still work as bigarray.cma still exists at the same place.

Yeah I confim. I was just wondering why this file has not been moved to stdlib?

ghost · 2018-04-18T14:56:55Z

Because it's not part of the stdlib :)

Stdlib.Bigarray, like all other stdlib modules, is compiled as Stdlib__Bigarray and ends up part of stdlib.cma/cmxa. bigarray.cma is only the deprecated bigarray library, which simply extends Stdlib.Bigarray with the deprecated functions.

shindere · 2018-04-18T15:01:13Z

Jérémie Dimino (2018/04/18 14:57 +0000):

Because it's not part of the stdlib :) `Stdlib.Bigarray`, like all other stdlib modules, is compiled as `Stdlib__Bigarray` and ends up part of `stdlib.cma/cmxa`. `bigarray.cma` is only the deprecated bigarray library, which simply extends `Stdlib.Bigarray` with the deprecated functions.

Okay thanks. I'm not sure what one will have to do re:this lib-dynlink-csharp test when the deprecated bigarray will be removed? Perhaps the tes will simply no longer need to load it and will continue to work?

ghost · 2018-04-18T15:03:54Z

Perhaps the tes will simply no longer need to load it and will continue
to work?

I believe so. It should even work right now

shindere · 2018-04-18T15:06:50Z

Jérémie Dimino (2018/04/18 15:03 +0000):

> Perhaps the tes will simply no longer need to load it and will continue to work? I believe so. It should even work right now

OK then I'll get rid of bigarray while porting to ocamltest Thanks a lot for your support!

…

shindere · 2018-04-18T15:52:40Z

@diml and @alainfrisch :

I am assumiing that unix.cma and unix.cmxs were loaded only to support
bigarray and that they are thus not needed any longer either. Is that
correct?

I hope stopping to load them does not defeat the purpose of the test itself
but I don't think so. Would just appreciate this to be confirmed.

ghost · 2018-04-18T16:03:37Z

I admit I don't really know what this test is doing, but I assume that it should indeed load a few files. Maybe the plugin should be updated to test a different function

shindere · 2018-04-18T16:06:38Z

Jérémie Dimino (2018/04/18 16:03 +0000):

I admit I don't really know what this test is doing, but I assume that it should indeed load a few files. Maybe the plugin should be updated to test a different function

Let's hope @alainfrisch will tell us. :)

Relevant changes in 4.07 - unix.c now #include <sys/ioctl.h> (since 852b595ff3), but an empty one is fine (only use is in caml_nun_rows_fd in an #ifdef TIOCGWINSZ), see ocaml/ocaml#1431 -> provide an empty ioctl via nolibc - bigarray is now part of the stdlib ocaml/ocaml#1685 -> libotherlibs.a is no longer needed (neither built nor linked)

Relevant changes in 4.07 - unix.c now #include <sys/ioctl.h> (since 852b595ff3), but an empty one is fine (only use is in caml_nun_rows_fd in an #ifdef TIOCGWINSZ), see ocaml/ocaml#1431 -> provide an empty ioctl via nolibc - bigarray is now part of the stdlib ocaml/ocaml#1685 -> libotherlibs.a is no longer needed (neither built nor linked) To support these changes, an empty sys/ioctl.h is added to nolibc. Also, `ocaml-freestanding.pc.in` now contains in `Libs`: -L${libdir} -lasmrum -lnolibc -lopenlibm (instead of ${libdir}/lib for each library). `configure.sh` adds -lotherlibs to these flags in the case that OCAML_GTE_4_07_0 is false. The `Makefile` treats libotherlibs.a as dependency only if OCAML_GTE_4_07_0 is false.

damiendoligez added stdlib high-priority labels Mar 30, 2018

Move bigarray to the stdlib

c264c55

mshinwell approved these changes Apr 9, 2018

View reviewed changes

Merge remote-tracking branch 'ocaml/trunk' into move-bigarray-to-the-…

6b71e33

…stdlib

mshinwell merged commit 32da45a into ocaml:trunk Apr 9, 2018

hannesm mentioned this pull request Jun 20, 2018

OCaml 4.07 support mirage/ocaml-solo5#39

Merged

anmonteiro mentioned this pull request Feb 11, 2019

4.08 compilation errors ocsigen/lwt#658

Closed

aantron mentioned this pull request Feb 14, 2019

bigarray compatibility library not linked ocaml/dune#1845

Closed

vicuna mentioned this pull request Mar 14, 2019

reversing the Unix and Bigarray dependency #6139

Closed

dinosaure mentioned this pull request Jun 17, 2019

Use Unsigned_long_val instead of Long_val in C stubs inhabitedtype/bigstringaf#32

Merged

Et7f3 mentioned this pull request Jan 22, 2023

Fix ocaml packages.megam NixOS/nixpkgs#212039

Merged

13 tasks

dra27 mentioned this pull request Jan 20, 2025

Move Dynlink into the Standard Library and use it in the toplevel #13745

Open

Conversation

ghost commented Mar 30, 2018

Overview

Limitation

Reviewing

Uh oh!

shindere commented Mar 30, 2018 via email

Uh oh!

ghost commented Mar 31, 2018

Uh oh!

dra27 commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshinwell commented Apr 4, 2018

Uh oh!

mshinwell commented Apr 9, 2018

Uh oh!

mshinwell commented Apr 9, 2018

Uh oh!

mshinwell commented Apr 9, 2018

Uh oh!

dbuenzli commented Apr 9, 2018

Uh oh!

gasche commented Apr 9, 2018

Uh oh!

shindere commented Apr 18, 2018

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

shindere commented Apr 18, 2018

Uh oh!

ghost commented Apr 18, 2018

Uh oh!

shindere commented Apr 18, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dra27 commented Apr 2, 2018 •

edited

Loading