-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open Extensible Types #5584
Comments
Comment author: @alainfrisch I like the proposal very much, but I'm not so fond of the proposed syntax.
type foo += Foo of int | Bar of string
|
Comment author: @lpw25 I agree that the proposed syntax is not very good. Regarding the above suggestions: For type declaratons, I think that the "type 'a foo = .." syntax would be fine. Another alternative would be "type > 'a foo", borrowing the ">" from polymorphic variants. Whichever syntax is used for type declarations, I think that it should definitely allow type abbreviations to be checked for their openness. For instance: type foo = bar should create a type abbreviation that is open if bar is open, but type > foo = bar should check that bar is open and create a type abbreviation. 2 and 3. I'm not sure about allowing multiple extensions to be declared at once, or allowing extensions to be declared within the type declaration, because: type foo = A | B | .. will be compatible with type foo = B | A | .. which could cause confusion. I also think that if it is too easy to create an open type and extensions that are equivalent to an ordinary variant type, then people will create them unnecessarily. Essentially, the additional runtime cost of using extensions compared to ordinary variants should be reflected by the syntax in some way. |
Comment author: @alainfrisch
type bar = foo
I agree with your remark about defining constructors directly within the type declaration. But I don't see the problem with multiple extensions: type foo = .. |
Comment author: @lpw25 For the rebinding it would be more like: "type foo += X = M.Y", which is probably fine. I think that having all of "type foo += A of int | B of string" "type foo += B of string | A of int" "type foo += A of int as equivalent definitions may cause confusion, because there aren't really any other features in OCaml that allow similar duplication. Such syntactic sugar might be better added using camlp4. |
Comment author: @garrigue I have not yet tried the patch, but the idea seems fine. type open foo = ... Since open is already a keyword there is no problem. type foo += A of int | B of bool Since the syntax follows the one for variants, there is no reason have people write more than necessary. For me the advantage of extensible types is double:
The real goal of polymorphic variants was to allow reusing the same constructor in different types, with fine grain pattern-matching, and this is orthogonal to this. |
Comment author: @lpw25 I have attached a new version of the patch (tested with revision 12464). The new version uses the syntax: type 'a foo = .. to define open types. To match this the behaviour has been changed so that open types are a new kind of type, rather than a property of types. The syntax for adding extensions has also been changed to: type 'a foo += To match this syntax, these have been changed to behave more like extensions to a type's definition, rather than declarations of constructors. Intuitively, they behave like: type 'a foo_new = 'a foo + A of 'a | B: int foo_new and can be used wherever the definition: type 'a foo_new = 'a foo = .. would be allowed. In the above definition, although A and B are defined in a single extension, they are separate members of a module signature so that the following definitions are equivalent to the one above: type 'a foo += A of 'a type 'a foo += B: int foo Extensions can be made private using the expected syntax: type 'a foo += private C of int They can also be rebound: type 'a foo += D = A The patch also changes the implementation to use Alain's suggestion of using an object tag instead of needing a new tag for extensions. I have added the extension to Ocamldoc and the other tools, but I did not add the new syntax to either camlp4 or the emacs mode, as I thought that was better left to someone who knows more about them. |
Comment author: @hcarty Is this patch still being considered for inclusion? I tried applying patch 2.0.0 against the trunk revision 12594 and it does not apply cleanly. |
Comment author: @lpw25 I don't know whether it's being considered for inclusion, but patch 2.0.0 was made for revision 12464 and should work with that one. I wasn't planning on rebasing the patch on a newer revision until OCaml version 4.0.0 is released. |
Comment author: @hcarty Will there be an update to this patch now that 4.00.0 is out? |
Comment author: @lpw25 The latest version of this patch (currently about 6 months behind trunk) can be found at https://github.com/lpw25/ocaml-open/. There is also a version built against 4.00.1 at https://github.com/lpw25/ocaml-open/archive/4.00.1+open_types.tar.gz |
Comment author: @garrigue This is a large patch, but actually a big part of it is about updating ocamldoc to handle extensible types... I've created a branch open_types in subversion, merging extension.patch-3.0.0. |
Comment author: @lpw25
I've attached a patch (shorter-transl-extension-constructor.patch) which makes transl_extension_constructor shorter by factoring out the record creation in each branch of the match. It is still quite a long function, but most of that is the logic for rebinding extensions which can't really be avoided.
Damn, I thought I caught all of those (excluding ocamldoc whose code does not obey the 80 column rule at all). |
Comment author: @garrigue Sorry for the slow reaction. Also, what about the reason to keep exceptions as an independent mechanism? |
Comment author: @lpw25
Possibly
There is a slight difference in representations of exception declarations and type extensions. The exception declarations contain a string with the name of the exception. This allows the name to be printed if the exception is not caught. Type extensions do not include such a string. So currently Note that the constructors themselves are already represented the same (using |
Comment author: @alainfrisch Wouldn't it be useful to keep the constructor name, for debugging purposes? I don't know precisely how this would be exported to the user, maybe with a special primitive that can be instantiated only on extensible types. I.e. the user would write: type 'a foo = .. val name: 'a foo -> string = "%caml_constructor_name" (The compiler would ensure that the primitive is only used at the proper types.) |
Comment author: @lpw25
The simplest way would probably be via a function in |
Comment author: @alainfrisch
Yes, but I assume its signature cannot be restricted to accept only values of extensible types, hence my proposal. Another approach would be to define a built-in supertype of all extensible types, and expose a single function working on that type: val name: extensible -> string ... name (x :> extensible) |
Comment author: @lpw25
As the use cases for this feature are just debugging, I'm not sure that it is worth the additional complexity. A run-time exception for misusing the function should be sufficient (and achievable since I don't think an object tagged block whose first parameter is a string appears elsewhere). |
Comment author: @lpw25 I've uploaded another patch (to be applied on top of the last one). This patch does 4 things:
These commits have also been added to the pull request, where they can be viewed as separate diffs. |
Comment author: @alainfrisch Jacques: I think you forgot to add ocamldoc/odoc_extension.ml to the branch. |
Comment author: @alainfrisch I'm testing the branch, not the latest patches, so maybe this doesn't apply: # exception A;; exception A # A;; - : exn = A # type foo = ..;; type foo = .. # type foo += A;; type foo += A # A;; - : foo = It could be useful to have the toplevel uses the same printing heuristics for extensible types as for exceptions. |
Comment author: @lpw25
That is indeed added by the latest patch, since it depends on storing the name in the "slot". It is slightly less aggressive than the exception printer (which kind of breaks abstraction when it cannot find the correct exception definition), but it will now print |
Comment author: @alainfrisch The patch represents type parameters with "core_type" instead of "string loc option" in the Parsetree. This was discussed in April 2013 on the wg-camlp4 mailing list ("Changes to the parsetree" thread). My position is still that it's a good change, but only if the parser accepts arbitrary core types in position of type parameters, and have the type-checker reject forms which are not allowed (the argument is that it reduces the "syntactic invariants" required for a parsetree to be printed and parsed backed). Moreover, it could certainly be useful to allow attributes on type parameters. As far as I remember, allowing arbitrary type expressions introduced some conflicts in the grammar, though. How is this change related to the current proposal? If there is no real need for it, I'd propose to dissociate this point from the patch. |
Comment author: @lpw25
I found it useful to be able to obtain the Note that, unlike a I think that it is also a more sensible representation. The parameter is represented with (restricted) type expression syntax and has a corresponding Also note that, in the presence of
That should certainly be easy after this change. |
Comment author: @alainfrisch Do you think it would be possible to tweak the grammar to accept all kinds of type expressions, then? It would be good that the type-checker checks for accepted forms anyway, to prevent weird behavior with badly buggy ppx rewriters for instance. |
Comment author: @lpw25
I'm not sure, I'm assuming that there is some kind of conflict. But it should certainly be possible to extend the grammar to allow things like attributes and extensions.
This is a good point, although it raises some much wider issues: what invariants does the type checker assume about its input from the parser and are they all checked? What error should be raised for a case that is only possible with a faulty ppx rewriter? Should there be a sanity check that encodes all the invariants and can be run before type-checking? |
Comment author: @garrigue I've merged merge-exceptions in the open_types branch, and added forgotten files. |
Comment author: @garrigue I do not think it is a good idea to accept arbitrary type expressions as formal parameters in a declaration. The question of attributes is of course orthogonal to that. |
Comment author: @lpw25
That's the idea.
Without searching properly for an actual example, I assume that there are probably a few places where particular parsetree does not correspond to a valid output of the parser. Do you think we should try and locate these assumptions and ensure that they are checked properly during type-checking? Perhaps we should have a "Illegal Parse Tree" error for such cases. |
Comment author: @alainfrisch
"Structural" invariants are now better expressed in the Parsetree. For instance, there are two different nodes for Pexp_fun and Pexp_function, while previously a kind of node could express forms that don't have a valid syntax. There are still some constraints such as Ptyp_poly and Pexp_poly which can only appear in specific contexts (documented in the parsetree.mli) -- I don't remember if the type-checker behaves correctly when the constraint is not satisfied. There are also a lot of side conditions, such as many lists being non empty, identifier being well-formed, etc.
This seems a good goal, and a first step is to document missing constraints in parsetree.mli. |
Comment author: @lpw25 Is there anything else I should be doing to help this get merged? I would really like to see it in 4.02.
I think I might work on a patch to give "illegal parse tree" errors instead of assertion failures in cases where parse tree invariants are not met, but I would prefer to keep that separate form this extensible variants patch. |
Comment author: @garrigue I think it's ready for merging, but I don't have much time to do it. |
Comment author: @lpw25 Patch made up-to-date again (extension.patch-4.0.1). This is against current trunk (r14708). |
Comment author: @garrigue Merged extension.patch-4.0.2 in trunk at revision 14737. |
Comment author: @gasche Congratulations! That was a lot of work, and we now have a nice feature. |
Comment author: @alainfrisch Indeed, this is great! Leo: can you propose a patch to the manual as well? |
Comment author: @lpw25
Sure. I'll have a look at that sometime in the next few weeks. |
Comment author: ybarnoy Awesome job! This is shaping up to be a terrific release. |
Original bug ID: 5584
Reporter: @lpw25
Assigned to: @garrigue
Status: closed (set by @xavierleroy on 2015-12-11T18:26:46Z)
Resolution: fixed
Priority: normal
Severity: feature
Fixed in version: 4.02.0+dev
Category: ~DO NOT USE (was: OCaml general)
Tags: patch
Related to: #6219
Monitored by: @gasche @lpw25 mehdi @diml @ygrek @yallop @hcarty @yakobowski @alainfrisch
Bug description
OCaml already has one open extensible type (exn), it would be useful to extend this and allow abstract types to be declared open.
This could be particularly useful when combined with GADTs.
Example syntax:
open type foo
extend foo with Constr of int
open type 'a bar
extend _ bar with Gadt1: int bar
extend char bar with Gadt2
I've attached a patch (tested with revision 12270) and more details can be found at:
https://sites.google.com/site/ocamlopen/
Note that this patch does not yet add these extensions to the camlp4 parser, and may break existing camlp4 code.
File attachments
The text was updated successfully, but these errors were encountered: