Skip to content

Make sure Bigarray shadows Stdlib.Bigarray#2256

Closed
ghost wants to merge 1 commit intotrunkfrom
unknown repository
Closed

Make sure Bigarray shadows Stdlib.Bigarray#2256
ghost wants to merge 1 commit intotrunkfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Feb 18, 2019

In trunk and 4.08, Stdlib.Bigarray shadows Bigarray (from the bigarray library). The reason is that the initial environment is constructed as follow:

env <- empty

env <- add_dir(env, stdlib_dir)
env <- open(env, Stdlib)

env <- add_dir(env, d1)
...
env <- add_dir(env, dn)

env <- open(env, M1)
...
env <- open(env, Mm)

Where dX are the include directories specified by the user via the -I option and MX are the implicitly opened modules specified via -open. This seems correct, however there is one catch: the bigarray library is installed in the same directory as the stdlib, which means that the open(env, Stdlib) shadows the toplevel Bigarray module :/

To workaround this issue, this patch reintroduces the names coming stdlib_dir after opening the Stdlib module, i.e. the initial environment is now constructed as follow:

env <- empty

env <- add_dir(env, stdlib_dir)
env <- open(env, Stdlib)
env <- add_dir(env, stdlib_dir)

env <- add_dir(env, d1)
...
env <- add_dir(env, dn)

env <- open(env, M1)
...
env <- open(env, Mm)

I tried but I didn't managed to write a test for this; in any test I wrote, Stdlib.Bigarray was still shadowing Bigarray even after this patch. I'm assuming that this is because the testsuite must use some special flags such as -nopervasives or -nostdlib in order to use a local stdlib. However, I did the following manual testing:

$ git clean -dfx
$ ./configure --prefix ~/code/ocamls/trunk
$ make world.opt -j20
$ make install
$ ocaml
# Bigarray.Array1.map_file

with trunk, I get a unbound value map_file error, while with this patch I properly get a deprecation message.

Limitation

If one uses compilation flags such as -nopervasives -open Stdlib for special purposes, then Stdlib.Bigarray will still shadow Bigarray. I don't have a good solution for this, except installing the various libraries in their own sub-directory as done in #1569, which would avoid this issue altogether.

@jeremiedimino
Make sure Bigarray shadows Stdlib.Bigarray
b65b6f9 
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
This was referenced Feb 18, 2019
Merged
Closed
@aantron aantron mentioned this pull request Feb 18, 2019
Closed
@damiendoligez
Copy link
Copy Markdown
Member

damiendoligez commented Feb 20, 2019

The limitation is serious, as it means that we cannot try an uninstalled compiler without getting some weird behaviors. And as you noticed it somewhat messes up our test suite. This means it's probably time to try reopening #1569 or something similar.

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 20, 2019

Personally, I would like #1569 to be reopened as it makes reasoning about all this a lot simpler and it is also beneficial for users. I can think of alternative ways, but that would be more complexity that might make things more difficult in the future.

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 21, 2019

I'm closing this since this is not a good fix. I sent an email on caml-devel to discuss how to proceed.

@ghost ghost closed this Feb 21, 2019
@aantron
Copy link
Copy Markdown
Contributor

aantron commented Feb 21, 2019

@diml Would you mind pinging this issue again with the number of the follow-on issue or PR, once it is open/reopened, so that subscribers can monitor it?

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 21, 2019

Sure, no problem

@ghost
Copy link
Copy Markdown
Author

ghost commented Feb 25, 2019

@aantron, we discussed this on caml-devel. The only additional feature provided by the Bigarray module from the bigarray library is ArrayX.map_file. Given that these functions have been deprecated since 4.06.0, it doesn't too bad that the bigarray library is no longer accessible. The official answer is to simply switch to Unix.map_file. You might also want to stop relying on the existence of the bigarray library, since this library will disappear in the future.

FTR, we have been slowly moving Bigarray into the standard library. It has been a long process, but it should hopefully be completely over in a version or two of the compiler. Starting with 4.08, you should consider that the bigarray library is deprecated.

@aantron
Copy link
Copy Markdown
Contributor

aantron commented Feb 25, 2019

Thanks, @diml. I'll adapt Lwt with some conditional compilation (since we need to support old versions of OCaml).

@damiendoligez
Copy link
Copy Markdown
Member

damiendoligez commented Mar 2, 2019

@aantron you should have a look at the discussion on #2263, maybe you won't need conditional compilation.

@aantron
Copy link
Copy Markdown
Contributor

aantron commented Mar 3, 2019

Thanks @damiendoligez!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

consider-for-release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@damiendoligez @aantron @nojb @jeremiedimino