Conversation
243d6bd to
3d4f72a
Compare
|
Thanks, @trefis. I plan to take a look at this soon (but not before next week). |
lpw25
left a comment
There was a problem hiding this comment.
I've looked over the translation parts so far. Looks good but there are a couple of bugs and some places where we could avoid allocating in the open struct ... end case.
268ae41 to
3d4f72a
Compare
lpw25
left a comment
There was a problem hiding this comment.
There are a couple of bugs, and a couple of bits that should be tidied up. Once they're fixed this should be good to go.
|
On the documentation front, I have a subpr here that adds some basic explanations and examples for the new features, if people wish to discuss the documentation separately. |
3440ed0 to
12ad467
Compare
|
I have rebased, answered all of @lpw25's comments, and included @Octachron's changes to the manual. |
12ad467 to
9857699
Compare
|
I pushed one last commit which updates the manual as discussed. |
Is the following intentional? # let rec x = let module M = struct let y = lazy x end in ();;
val x : unit = ()
# let rec x = let module M = struct include struct let y = lazy x end end in ();;
val x : unit = ()
# let rec x = let module M = struct module M = struct let y = lazy x end end in ();;
val x : unit = ()
# let rec x = let module M = struct open struct let y = lazy x end end in ();;
Line 1, characters 12-74:
1 | let rec x = let module M = struct open struct let y = lazy x end end in ();;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: This kind of expression is not allowed as right-hand side of `let rec' |
e6a8a82 to
1bbac36
Compare
|
I guess it is: I guess you might want to make |
|
From the CI, it looks like |
| else | ||
| type_args [] [] ty (instance ty) ty sargs [] | ||
| let ty = funct.exp_type in | ||
| if ignore_labels then |
There was a problem hiding this comment.
just curious, do we have an official code style for ocaml compiler source code now? are we using ocamlformat?
| | Texp_extension_constructor _ -> | ||
| () | ||
| | Texp_open (_od, e) -> | ||
| iter_expression e |
There was a problem hiding this comment.
should we iter _od here? also we should iter Tstr_open?
There was a problem hiding this comment.
I updated this and Tstr_open to do the same thing as with includes.
typing/typedtreeMap.ml
Outdated
| Texp_unreachable | ||
| | Texp_extension_constructor _ as e -> | ||
| e | ||
| | Texp_open (od, exp) -> Texp_open (od, map_expression exp) |
There was a problem hiding this comment.
should we map over module expression of od here? Also Tstr_open
| match m.mod_desc with | ||
| Tmod_ident _ -> Alias | ||
| | Tmod_constraint (m,_,_,_) -> pure_module m | ||
| | _ -> Strict |
There was a problem hiding this comment.
can we list out all other cases here explicitly?
There was a problem hiding this comment.
I just moved this code from Translmod to Translcore. I didn't actually write it. So I'd rather not update it.
| (** Extracts the list of "value" identifiers bound by a signature. | ||
| "Value" identifiers are identifiers for signature components that | ||
| correspond to a run-time value: values, extensions, modules, classes. | ||
| Note: manifest primitives do not correspond to a run-time value! *) |
There was a problem hiding this comment.
I know this is copied over from another file, but I'd like to know what "manifest primitives" means. Thanks.
There was a problem hiding this comment.
I think reading this as "externals" is acceptable.
manual/manual/refman/exten.etex
Outdated
| \begin{caml_example}{verbatim} | ||
| module M = struct | ||
| open struct type 'a t = 'a option = None | Some of 'a end | ||
| let x : int t = Some 1 |
There was a problem hiding this comment.
add one whitespace before let
1bbac36 to
33ec63e
Compare
lpw25
left a comment
There was a problem hiding this comment.
Good to merge once CI passes.
|
Thanks, @trefis! |
|
Thank you for the work @trefis! |
|
There's a bug in this PR. |
This is a refresh of #1506 which includes the restrictions that were discussed during the last developer meeting and since then on github (cf. #1506 (comment)).
More precisely:
openin signatures is changed to accept extended module pathsopenin structures, as well aslet openin expressions, are extended to accept any module expressionlet openin classes and class types is unchanged: before extending it we would need to add support forlet module, which is complicated. Cf. the discussion on MPR#6271M.(expr),M.(pat),M.[expr], etc.) are still restricted to module paths.Things to note:
Rec_check, but I didn't spend to much time on it, and it is untested. So it's likely to be wrong. It'd be nice if @gasche or @yallop could have a look.opento accept arbitrary module expression #1506 as they were, and updated the output in a separate commit, so it's easier to spot the differences between the two PRs