Skip to content
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

Get extension slot #273

Merged
merged 6 commits into from Nov 27, 2015
Merged

Get extension slot #273

merged 6 commits into from Nov 27, 2015

Conversation

ghost
Copy link

@ghost ghost commented Oct 30, 2015

This patch adds support for [%extension_slot <path>]. The compiler expands it to the slot of the extension constructor denoted by <path>. For instance:

type t = ..
type t += A of int

let () = assert (Obj.extension_slot (A 42) == [%extension_slot A])

The motivation for this request is ppx rewriters that needs to register a value associated to and extension constructor, such as what sexplib does for exceptions, and should do soon for all open types.

Currently the only way to get the extension slot is to build a dummy value, which is difficult to do in a ppx rewriter. In sexplib for instance we have a module Exn_magic that is full of Obj.magic to deal with that.

@alainfrisch
Copy link
Contributor

If we go this way, I would argue in favor of a new dedicated type to represent extension slot (e.g. Obj.extension_slot). This type could also serve as the return type for the existing Obj.extension_slot function. Obj.extension_id and Obj.extension_slot would take a value of this type, avoiding the current check of well-formedness (needed since they can currently be applied on arbitrary values). This would change they current behavior, but also make them more robust and efficient.

As for the choice of the syntax: I'm not very fond of the use of the [%...] extension syntax, but I don't see a better alternative.

@ghost
Copy link
Author

ghost commented Nov 10, 2015

I agree with the proposed changes. I didn't do it initially to avoid breaking the API but it's better to have a specific type indeed. I'll update the patch.

@alainfrisch
Copy link
Contributor

I'm in favor of accepting this PR, but I'd like to get others' opinion on the use of an extension node for something actually interpreted by the type-checker. Should we rather use some new ad hoc syntax?

A minor technical glitch is that since ocamldep does not look inside extension nodes, it won't report a dependency created purely by this new form.

An alternative way could be to rely on the existing Obj.extension_slot function, which would be implemented as a primitive with some custom behavior in the code generator to detect when the argument has a known head constructor, so that one could write Obj.extension_slot (Foo (assert false)) and the assertion would not be evaluated. There are precedents where primitives exposed as functions don't follow the normal call-by-value semantics (Pervasives.(&&)); I think this might be fine for such a function which is supposed to be used by code generators mostly, as long as this is properly documented. One problem is that writing Obj.extension_slot (Foo (assert false)) works only if the constructor takes exactly one argument, though.

@ghost
Copy link
Author

ghost commented Nov 12, 2015

I'm in favor of accepting this PR, but I'd like to get others' opinion on the use of an extension node for something actually interpreted by the type-checker. Should we rather use some new ad hoc syntax?

Personally I'm fine with using extension points for things that are more expert details of the language. It avoids introducing keywords while still making important features available.

But if someone comes up with a combination of keywords that make sense I have nothing against it.

A minor technical glitch is that since ocamldep does not look inside extension nodes, it won't report a dependency created purely by this new form.

Good point, I didn't thought about it. Although I think that if we accept that the compiler interprets [%extension_slot], then ocamldep should do the same. I'm updating ocamldep in this patch.

An alternative way could be to rely on the existing Obj.extension_slot function, [...]

I prefer [%extension_slot] for several reasons:

  • it obsoletes Callback.register_exception, which is not a huge win but still makes things slightly cleaner
  • Foo (assert false) is a bit confusing. Even if it'd be mostly generated, when people look at the generated code they'll be confused (until they read the doc for extension_slot)
  • the rule for && is used in a lot of languages so it's more intuitive

@gasche
Copy link
Member

gasche commented Nov 15, 2015

I would suggest to add a constructor for this in the parsetree (which would be parsed as an extension point, and reprinted as such). ocamldep (and other tools) works on the parsetree, so that would automatically handle this.

@gasche
Copy link
Member

gasche commented Nov 18, 2015

Xavier would like us to find a better name than "extension slot" for this concept, described by Leo and Mark in stereo as "the thing that gets allocated when we declare a new extension constructor".

If we have a better name, this can go in trunk. (Before mid-december would be important, as after that we will have the feature freeze for 4.03)

(Of course I think it is fine to keep Obj.extension_slot available for API compatibility, but also having Obj.<better_name> would be nice.)

@damiendoligez
Copy link
Member

IIUC, this "extension slot" is extremely similar to an object ID, so I'd propose "extension ID" or "constructor ID".

@gasche
Copy link
Member

gasche commented Nov 23, 2015

Would "extension constructor id" be too verbose? (Possibly abbreviated extcon_id?)

( @damiendoligez (alone) is going to like this next suggestion: if we use excon_id (please ignore the dubious accidental pun) we get to not specify whether it is an "extension constructor" or an "exception constructor" )

@trefis
Copy link
Contributor

trefis commented Nov 23, 2015

Would "extension constructor id" be too verbose? (Possibly abbreviated extcon_id?)

I think it's fine to be verbose for such (I assume) rarely used features. I dislike extcon_id.

@ghost
Copy link
Author

ghost commented Nov 23, 2015

One case where this can be used a lot is when one wants to build values of an open type from C as all constructors have to be registered using this. That said I prefer an explicit name and I'm fine with extension_constructor_id.

I just have one reservation against using id in the name as the value referred by [%extension_constructor_id ...] is a block with tag Obj.object_tag that contains an integer id so it might be confusing...

On the other hand the manual describe this value as an exception identifier (for exceptions) so maybe it's fine. We can use this API in Obj:

val extension_constructor_id : _ -> extension_constructor_id
val extension_constructor_unique_integer_id : extension_constructor_id -> int
val extension_constructor_name : extension_constructor_id -> string

@gasche
Copy link
Member

gasche commented Nov 23, 2015

Given the API you describe extension_constructor_repr seems more natural. This is the runtime representation of a dynamically-created constructor of an extensible variant type.

@damiendoligez
Copy link
Member

Except that a constructor is not a value, so the analogy with Obj.repr doesn't work.

@gasche
Copy link
Member

gasche commented Nov 24, 2015

I didn't think of the conflict with Obj.repr (no analogy was harmed in my proposal), it's unfortunate because "representation" is appropriate here. Robert Harper speaks of "dynamically-classified exception" for the language feature of creating new exceptions at runtime (see for example this blog post; I think, in passing, that he still wrongly believes that OCaml does the wrong thing there). Maybe extension_constructor_classifier in his honor?

@gasche
Copy link
Member

gasche commented Nov 24, 2015

dynamic_constructor_classifier? dynamic_extension_classifier?

@xavierleroy
Copy link
Contributor

We should leave all choices of function names to @mshinwell :-) More seriously, I'm fine with "extension_constructor_id".

@mshinwell
Copy link
Contributor

How about just using "extension_constructor"? The values are exactly the runtime representations of such.

Jeremie Dimino added 6 commits November 27, 2015 17:52
So that we can create constants of type [extension_constructor]
Rename Obj.extension_slot to Obj.extension_constructor as well
Translate [%ocaml.extension_constructor <path>] to the
runtime-representation of the extension constructor denoted by
<path>. This allows one to get the extension constructor without
having to create a dummy value.
Make ocamldep inspect the payload of [%extension_constructor]
@ghost
Copy link
Author

ghost commented Nov 27, 2015

I updated the patch to use [%extension_constructor]

gasche added a commit that referenced this pull request Nov 27, 2015
@gasche gasche merged commit debae3b into ocaml:trunk Nov 27, 2015
@gasche
Copy link
Member

gasche commented Nov 27, 2015

Thanks all for this exciting naming session! Naming is fun.

mshinwell added a commit to janestreet/ocaml that referenced this pull request Sep 30, 2020
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Oct 5, 2021
* Make Var_in_binding_pos satisfy Bindable and use it instead of Bindable_variable_in_terms

* Remove middle_end/flambda2/naming/bindable_variable_in_terms.ml{,i}

* Rename naming/ -> nominal/

* Introduce bound_identifiers/ directory

* Rename modules in bound_identifiers/

* Reformat

* Update CODEOWNERS
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants