cc @alainfrisch

May I have a round of review?

This would be awesome for me! I'm implementing a sort of typeclass thing, and it'd be great to not have to call the typeclass implementation functors, except in open.

(right now, I have something like

(* in Result *)
module Monad(E: Type) = struct
  type error = E.t;
  ...
end

(* in cafec/parse/error.re *)
module Monad_result = Result.Monad(sig type nonrec t = t end);

(* in cafec/parse/parse.re *)
open Error.Monad_result;

I'd rather write

(* Result is the same *)

(* in cafec/parse/parse.re *)
open Result.Monad(sig type t = Error.t end)

)

There's a bug in this patch. I've installed this and switched to it, and attempted to install jbuilder:

$ opam install jbuilder
The following actions will be performed:
  ∗  install conf-m4   1                      [required by ocamlfind]
  ∗  install ocamlfind 1.7.3                  [required by jbuilder]
  ∗  install jbuilder  1.0+beta16
===== ∗  3 =====
Do you want to continue ? [Y/n] y

=-=- Gathering sources =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
[default] https://opam.ocaml.org/archives/ocamlfind.1.7.3+opam.tar.gz downloaded
[default] https://opam.ocaml.org/archives/jbuilder.1.0+beta16+opam.tar.gz downloaded

=-=- Processing actions -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
∗  installed conf-m4.1
∗  installed ocamlfind.1.7.3
[ERROR] The compilation of jbuilder failed at "ocaml bootstrap.ml".

#=== ERROR while installing jbuilder.1.0+beta16 ===============================#
# opam-version 1.2.2
# os           linux
# command      ocaml bootstrap.ml
# path         /home/nicole/.opam/4.06.0+pr1506/build/jbuilder.1.0+beta16
# compiler     4.06.0+pr1506
# exit-code    2
# env-file     /home/nicole/.opam/4.06.0+pr1506/build/jbuilder.1.0+beta16/jbuilder-6972-8b8a2d.env
# stdout-file  /home/nicole/.opam/4.06.0+pr1506/build/jbuilder.1.0+beta16/jbuilder-6972-8b8a2d.out
# stderr-file  /home/nicole/.opam/4.06.0+pr1506/build/jbuilder.1.0+beta16/jbuilder-6972-8b8a2d.err
### stdout ###
# /home/nicole/.opam/4.06.0+pr1506/bin/ocamllex.opt -q src/sexp_lexer.mll
# /home/nicole/.opam/4.06.0+pr1506/bin/ocamllex.opt -q src/meta_lexer.mll
# /home/nicole/.opam/4.06.0+pr1506/bin/ocamldep.opt -modules src/action.ml src/action_intf.ml src/alias.ml src/ansi_color.ml src/arg_spec.ml src/artifacts.ml src/bin.ml src/build.ml src/build_interpret.ml src/build_system.ml src/clflags.ml src/cm_kind.ml src/config.ml src/context.ml src/file_tree.ml src/findlib.ml src/future.ml src/gen_meta.ml src/gen_rules.ml src/glob_lexer.boot.ml src/import.ml src/install.ml src/io.ml src/jbuild.ml src/jbuild_load.ml vendor/boot/jbuilder_opam_file_format.ml vendor/boot/jbuilder_re.ml src/js_of_ocaml_rules.ml src/lib.ml src/lib_db.ml src/loc.ml src/log.ml src/main.ml src/merlin.ml src/meta.ml src/meta_lexer.ml src/ml_kind.ml src/mode.ml src/module.ml src/module_compilation.ml src/ocaml_flags.ml src/ocamldep.ml src/odoc.ml src/opam_file.ml src/ordered_set_lang.ml src/package.ml src/path.ml src/sexp.ml src/sexp_lexer.ml src/string_with_vars.ml src/super_context.ml src/top_closure.ml src/utils.ml src/utop.ml src/vfile_kind.ml src/watermarks.ml src/workspace.ml > boot-depends.txt
# /home/nicole/.opam/4.06.0+pr1506/bin/ocamlopt.opt -w -40 -o boot.exe unix.cmxa boot.ml
### stderr ###
# Fatal error: exception Invalid_argument("index out of bounds")



=-=- Error report -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
The following actions failed
  ∗  install jbuilder 1.0+beta16
The following changes have been performed
  ∗  install conf-m4   1
  ∗  install ocamlfind 1.7.3

@ubsan: I believe this is not caused by the patch itself. During a few weeks, trunk broke jbuilder (see #1493). This is now fixed, but I believe the present PR was submitted from a broken-jbuilder trunk version, and it needs to be rebased on the current trunk (which correctly builds jbuilder).

@gasche ah, okay :) thanks!

on another thing - the paper doesn't support local opens, apparently, but I'd really love local opens to support this as well, for purposes similar to typeclasses.

module type Print = sig
  type t
  val print: t -> unit
end
module Print_int: Print with type t = int = struct
  ...
end
module Print_list(P: Print): Print with type t = P.t list = struct
  ...
end

let print_list_of_int lst = Print_list(Print_int).(print lst)

@ubsan Thanks for your comment!

Extension on local open introduces grammar ambiguity. I didn't look closely or try to remove ambiguities because at first I thought such use case is rare. However, one reviewer and you mentioned it would be good to add support on local open as well. I can definitely look into this again.

@objmagic thanks <3

I think it's not really an ideal time to restart this whole discussion, after @objmagic was told that the feature was discussed and approved at the developers' meeting, and subsequently spent significant effort on implementation. While it's always fine to raise new technical issues, the objections seem to be old ones, and rather "soft" than technical, which makes it difficult to reach a definite conclusion.

Currently, since open only operates on values, it is compatible with more than one mental model (e.g. as a non-exporting version of include, or as a static operation on environments). The fact that extending it means committing to one of those models (and not the one you prefer) isn't a strong argument in itself. And while people could conceivably be confused by pretty much any change, I haven't encountered anyone who's at all confused by what's proposed here.

after @objmagic was told that the feature was discussed and approved at the developers' meeting

That only happened a couple of weeks ago, and I don't think Gabriel accurately reflected the discussion at the meeting, where I believe I already raised these points. I have also raised them with @objmagic in person in the past.

If everyone else is in favour, then as I said, I'm not going to try and stop it, but I thought that the public discussion on this feature should include my objections. (This was at least partly motivated by a conscious attempt to try to move more of these discussions into the public domain).

Currently, since open only operates on values, it is compatible with more than one mental model

The problem with signatures indicates that it is not compatible with both mental models. If open were a non-exporting version of include then it would take a signature when used in signatures. This is the inconsistency that I think will be confusing, it also means that this feature is only a partial solution to the problem of local definitions.

after @objmagic was told that the feature was discussed and approved at the developers' meeting

That only happened a couple of weeks ago

No, I told him the day of the meeting (November last year).

I thought that the public discussion on this feature should include my objections. (This was at least partly motivated by a conscious attempt to try to move more of these discussions into the public domain).

I agree that having the objections & discussion publicly available is a good idea. My only real objection is to the timing.

My only real objection is to the timing.

Understandable, but to be fair it's not like I'm making new design objections at the last minute, I'm just reiterating publicly design objections that I've already made privately and consistently to all parties involved.

I'm just reiterating publicly design objections that I've already made privately and consistently to all parties involved.

Nobody objects to that, of course. But you're also planning to implement and propose a competing design, which is a reasonable thing to do before consensus is reached, but not so much afterwards, unless new issues come to light.

But you're also planning to implement and propose a competing design, which is a reasonable thing to do before consensus is reached, but not so much afterwards, unless new issues come to light.

Well, as I said, I'm going to do that even if this feature is already released because it addresses important use cases not handled by this feature. I don't think the overlap is likely to cause users any confusion because, whilst they can be used to solve the same problems, the mechanisms they use are very different from a user's perspective.

Thank you for posting your view, @lpw25. I believe at least some of the support this feature has received (including my own) stems from the fact that we had no idea you had reservations about it, which only stresses how important it is to have a transparent process.

@lpw25 do you think this can get us into similar issues as the recent backwards-incompatible module type change?

And in terms of a meta-discussion:
a. I do appreciate all the effort @objmagic put into this specific PR.
b. I really think we should have either a recording or the notes of developer meetings posted online.
c. A PR such as this should probably be posted as an RFC before implementation begins, so that this discussion could take place publicly without the bias of sunk cost. Only some features seem to be suggested this way, probably because there's no explicit procedure specifying it.

Should we start a discussion on Discourse about procedures for these things? Other languages seem to have very specific procedures in place for RFCs and such.

do you think this can get us into similar issues as the recent backwards-incompatible module type change?

My objections are purely on the design side. There are no technical issues with this change, nor any backwards compatibility concerns.

And in terms of a meta-discussion:

PRs are not the place for meta-discussion. Please use one of the other forums (e.g. discuss, caml-list) for such discussions.

I don't think the eventually of another way to do something similar should stop us from merging this PR. What it does is clean from a semantical point of view.

A note on private annotations:

• This was part of the object system a long time ago, but disappeared in 2.00.
• At the time, @xavierleroy was strongly opposed to this kind of "ad hoc" scoping, but he may have changed since.
• Since 2.00, private in classes has a different meaning.

For these reasons, I'm not stopping you from making a proposal in that direction (actually at least then I thought it was handy), but there is no guarantee that it would be accepted.

Just in case somebody wants to work on the "private" approach: I did some quick experiment with them on #682 and #683.

Notes from today's caml-dev meeting:

• Let's restrict to open as structure items and local opens (but not as signature items).
• @trefis will have a look at the implementation

Are there any notes of the caml-dev discussion?

I think it would be better to keep the signature version, too, both for symmetry/consistency, and because it makes it possible to give better signatures in some cases. For example, the following module

type t = T
open Set.Make(String)
let union_all : t list -> t = List.fold_left union empty

can be given a signature that matches the implementation very directly:

type t = T
open Set.Make(String)
val union_all : t list -> t

Without extended open in signatures you have two choices:

1. make up a name for the type:

   type t = T
   type made_up := Set.Make(String).t
   val union_all : made_up list -> made_up

2. expand all aliases:

   type t = T
   val union_all : Set.Make(String).t list -> Set.Make(String).t

Both of these seem worse than the version with open.

@yallop The comment was a bit short indeed.
Actually what was decided was that the signature case would benefit from a clearer syntax, as the use cases are more restricted, and this would be better reached by an independent PR.
See #2122 for this specific feature.
You are of course welcome to comment on that.

@garrigue: yes, I've seen #2122, and my comment above makes use of the := syntax from that PR. As the comment also says, := is a worse alternative in some situations, since it requires making up a fresh name where extended open does not.

So, even if #2122 (which I quite like) is merged, I still think it would be better to stick with the original proposal from this PR, and extend open uniformly in both structures and signatures.

Sorry, seems I didn't read your post to the end.
There were 3 criticisms for extending open in signatures:

• there is no symmetry with include, which has a different meaning in signatures
• allowing to write possible side-effecting structures in signatures is confusing (module type of being seen as a special case, that we would rather remove)
• Signature local bindings #2122 carefully avoids the avoidance problem, which could be particularly confusing in signatures

I'm not sure about your example of of open on paths containing applications.
It is already ok in types, so I'm not sure the possible side-effect would be a problem.
We would still have to restrict it to non-generative functors to avoid the avoidance problem.

I'm personally not keen on open with arbitrary module expressions in signatures. Essentially, I don't think that side-effecting expressions should appear in positions where the side-effects won't be executed.

However, this position is complicated by a number of issues with OCaml's current module system. OCaml's notion of a pure module expression is a path, but this is simultaneously too restrictive -- I would like to also include structures whose elements are all defined by aliases and functors whose result is a pure module expression -- and too permissive -- in some positions paths are allowed to include applications of side-effecting functors. I have plans, which were discussed at the meeting, to address the first of these issues by extending paths to a more general notion of pure module expression. The second issue is hard to address backwards-compatibly, especially without tracking side-effects in types, so those will probably remain in an awkward middle ground.

So I think that open in signatures should be restricted to paths -- including applicative functor applications. Once the notion of path is extended this will include structures containing only aliases. I think that will cover all the useful cases for open in signature. It also obeys the basic idea that side-effect expressions should not be allowed in signatures -- modulo the case of functor applications where OCaml is already a bit off.

This is more restrictive than the current patch because it doesn't allow structures that define values, module coercions or generative functor applications, but I think those are both quite dubious in signatures and not really useful for anything.

I mostly agree with @lpw25 on this: it's not especially useful for open in signatures to allow completely arbitrary module expressions, but it is useful to support applicative functor application and structures built from type aliases and other supported elements. (Aside: I think it'd be good to choose a name other than "path" for this restricted set of module expressions.)

values, module coercions or generative functor applications, but I think those are both quite dubious in signatures and not really useful for anything.

Transparent ascription (i.e. the operation M <: S that retains type equalities from M) could be useful as the argument of open.

Aside: I think it'd be good to choose a name other than "path" for this restricted set of module expressions.

Yes, I've been calling them something like "pure module expressions" when working on the modular implicits specification.

Transparent ascription (i.e. the operation M <: S that retains type equalities from M) could be useful as the argument of open.

Yes, I didn't mention it, but adding transparent ascription to the pure module expressions was also discussed at the meeting. You can see the specification that I was using here, although the transparent ascription is somewhat hidden because it is included as part of module aliases -- other uses of transparent ascription can be simplified away so they are not in the formal spec.