Fixes for 4.07

[damiendoligez]

For Hashtbl in src_tests, I'm not sure this is the best fix. You should ask @diml or the other contributors of ocaml/ocaml#1010.

[gasche]

The first patch is clearly necessary correct, and remains compatible with older OCaml versions. Thanks!

The second patch indeed is worrying. We probably have to put it under a version-condition, as it will break under older systems, and it also suggests that everyone using %import on stdlib modules will get something broken. We probably have to do something here.

@damiendoligez, could you split the two patches into two PRs, so that I merge the first and worry about the second one?

[ghost]

I'm not very familiar with way ppx_import works, but I'm surprised the second commit is needed. By following the aliases in the typing environment one should automatically end up to Stdlib__hashtbl without having to manually resolve the aliases.

[gasche]

I'm not very familiar either, but I think we miss the "following aliases in the environment" component, and that indeed (assuming it isn't already done) would be a more robust solution to the problem. For reference, the current environment-lookup code is there, but I haven't analyzed it yet.

[xguerin]

@gasche You most likely nailed it. It does not look like we are handling type aliasing cases when parsing the CMIs in the current environment. That's a missing feature; the second patch is not needed.

[ghost]

If I read the code correctly, ppx_import doesn't type the input at all and simply assumes that in [%import: A.B.C.t], A is a compilation unit name.

[ghost]

Without changing everything, a simple solution would be to ask the compiler libs for the initial environment, and then do the lookup in this environment rather than read the cmi file manually.

[whitequark]

If I read the code correctly, ppx_import doesn't type the input at all and simply assumes that in [%import: A.B.C.t], A is a compilation unit name.

Correct.

Without changing everything, a simple solution would be to ask the compiler libs for the initial environment, and then do the lookup in this environment rather than read the cmi file manually.

I believe this is the right solution.

[damiendoligez]

I've removed the first commit from this PR and kept the problematic one, since the discussion about it is happening here. I've submitted #24 with the non-problematic commit.

[gasche]

I made a first attempt this morning at using the type-checker to resolve paths. This doesn't completely work yet (WIP). The first attempt is the commit daf4cd5819f5963f33ae136a1115859f7c806c89, which builds an initial typing environment when [%import ] is processed and uses Typetexp.find_{type,modtype} to find the type or module type declaration corresponding to the given lid.

This first patch does not pass the testsuite, because not all longidents used by ppx_import are actually valid (type-correct) identifiers. The testsuite contains this example:

stuff.ml:

module type S = sig
  type t = ...
end

use.ml:

[%import Stuff.S.t]

Querying a member of a type signature in this way is not valid in OCaml, although it makes sense. To support this, we will have to be more elaborate, and mix type-environment lookup with manual splitting of the path and exploration of the returned signature.

[gasche]

In #25 I propose a more elaborate implementation that uses the type-checker in the global environment, but also supports sub-module-type names in paths as ppx_import does today.

[gasche]

This can be closed now that #25 has been merged.