Skip to content

Add injectivity annotations#9500

Merged
garrigue merged 16 commits into
ocaml:trunkfrom
garrigue:injectivity-annotations
Jun 15, 2020
Merged

Add injectivity annotations#9500
garrigue merged 16 commits into
ocaml:trunkfrom
garrigue:injectivity-annotations

Conversation

@garrigue

Copy link
Copy Markdown
Contributor

Add injectivity annotations on abstract types, to allow using them in GADTs or constrained parameters.

The syntax is as suggested:

type !'a

When combined with a variance, the variance comes first:

type +!'a t

See examples in testsuite/tests/typing-misc/injectivity.ml

@gasche

gasche commented Apr 27, 2020

Copy link
Copy Markdown
Member

Note: when we discussed this during a meeting, I suggested that you and @lpw25 write a short document that would explain, with examples, the current need for (and uses of) the feature (including the examples from Oleg, and the "injectivity witnesses" that @lpw25 mentioned). I think this would be very useful to follow what's going on -- I understand the general feature and I think agree with @lpw25 it is simple and nice to expose it, but I haven't been able to follow the recent practical interest in it.

@yallop

yallop commented Apr 27, 2020

Copy link
Copy Markdown
Member

I think this means that #5985 can finally be closed.

@yallop

yallop commented Apr 27, 2020

Copy link
Copy Markdown
Member

What's the situation with GADTs? I haven't looked at the implementation, but gave the branch a quick try and was surprised to discover that injectivity annotations for some indexes are rejected:

# type (!_, !_) eql = Refl : ('a, 'a) eql;;
Line 1, characters 0-39:
1 | type (!_, !_) eql = Refl : ('a, 'a) eql;;
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: In this definition, expected parameter variances are not satisfied.
       The 1st type parameter was expected to be injective invariant,
       but it is unrestricted.

@garrigue

Copy link
Copy Markdown
Contributor Author

@yallop There is no point in giving injectivity annotations for "nominal" (i.e. records and sums, including GADTs and extensible ones) types: their parameters are always injective by definition.

@yallop

yallop commented Apr 29, 2020

Copy link
Copy Markdown
Member

Understood. But since the annotations are correct, even if pointless, I think it's best to accept them when they're given.

@yallop

yallop commented Apr 29, 2020

Copy link
Copy Markdown
Member

In particular, I think this behaviour, where a variant definition matches an injective signature, but can't be marked as injective directly, is surprising:

# module M : sig type !_ t end = struct type _ t = X end;;
module M : sig type !_ t end

# module M : sig type !_ t end = struct type !_ t = X end;;
Line 1, characters 38-51:
1 | module M : sig type !_ t end = struct type !_ t = X end;;
                                          ^^^^^^^^^^^^^
Error: In this definition, expected parameter variances are not satisfied.
       The 1st type parameter was expected to be injective invariant,
       but it is unrestricted.

@garrigue

Copy link
Copy Markdown
Contributor Author

@yallop The last commit ignores injectivity annotations for nominal types, so this fixes your problem.

@lpw25

lpw25 commented Apr 29, 2020

Copy link
Copy Markdown
Contributor

You can see the injectivity witnesses people use here:

https://ocaml.janestreet.com/ocaml-core/latest/doc/base/Base/Type_equal/module-type-Injective/index.html

I'm not sure whether any uses of it are in our open source code, but I think how you use it is fairly obvious. I'll try and dig up an example of why you might use it when I have some time.

@garrigue

garrigue commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

This PR is now ready for reviewing.

It implements injectivity annotations on type parameters, using the suggested syntax !. Injectivity comes after variance, so one can write type +!'a t.

The main motivation is solving the famous #5985, explained in my OCaml 2013 paper. In particular, this allows to define GADTs where parametric abstract types appear as index. See the nominal types RFC or Oleg's recent mail on the caml-list for examples of this problem.

Injectivity annotations do not make sense for nominal types (records or sum types), since all their parameters are injective by definition. For them, annotations are silently ignored.

For type abbreviations (public or private), the injectivity of parameters can be inferred from the body of the type, and annotations are only checked.

For abstract types and private row types, injectivity annotations should be satisfied by the actual implementation. They are kept in the type definition, and checked through module subtyping. Note that for private row types, injectivity can also be inferred in some cases.

For concrete examples, see the file testsuite/tests/typing-misc/injectivity.ml.

The implementation has two parts.

  • The first one, about injectivity of normal type parameters is only about adding syntax to express injectivity, and mapping it to the already existing injectivity bit of variances. There was almost nothing to change internally, since injectivity is already inferred. The main points are parsing (we allow both + ! and +!, without introducing a specific lexeme for +!), and printing (i.e. print injectivity annotations, but only where they are useful).
  • The second part concerns the injectivity of constrained type parameters in type abbreviations. In order to integrate them smoothly, their injectivity is now inferred (this was not the case before, except when the parameter itself appeared in the type), i.e. such a parameter is deemed injective if it appears at an injective position in the body of the type, or if all its type variables are injective. As a result, one never needs to annotate explicitly a type abbreviation, so that injectivity annotations are only shown when printing abstract types and private row types.

@garrigue

Copy link
Copy Markdown
Contributor Author

@lpw25 Thank you, this is clear enough indeed.
I suppose everybody would be happy not to have to do this kind of gymnastic.
So adding injectivity annotations seems to be a reasonable first step, all the more as the compiler already tracks it.

@garrigue

Copy link
Copy Markdown
Contributor Author

This PR is a bit strange, as most of the effort seems to be going into constrained type parameters in type abbreviations, which is a very minor feature.
I have added a bit more code, to get maximal expressiveness: it is possible to write types in a way such that one can infer all the possible injectivity, even with constrained type parameters. On the other hand, writing it in a different way may lose injectivity, but does it really matter when those types a rare enough.

@garrigue garrigue requested a review from yallop April 30, 2020 12:18

@yallop yallop left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very welcome change!

I believe the code to be correct, but please see the comments attached to this review for some suggestions for small improvements to the interface, implementation and documentation.

Comment thread typing/printtyp.ml
Comment thread parsing/parser.mly
Comment thread typing/outcometree.mli Outdated
Comment thread typing/typedecl_variance.ml
Comment thread manual/manual/refman/typedecl.etex
Comment thread Changes Outdated
Comment thread manual/manual/refman/typedecl.etex Outdated
@gasche

gasche commented Jun 10, 2020

Copy link
Copy Markdown
Member

@yallop I wonder if you are satisfied with @garrigue's handling of your points, so that the PR can move further. (This said, it needs a full code review to move further, and I don't know who is volunteering for that, I'm currently not.)

@lpw25 lpw25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks correct to me and I'm in favour of the feature.

Comment thread manual/manual/refman/typedecl.etex Outdated
are inferred from its definition, and the variance annotations are
only checked for conformance with the definition.

Injectivity annotations only make sense for abstract types and private

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Injectivity annotations only make sense for abstract types and private
Injectivity annotations are only necessary for abstract types and private

@yallop

yallop commented Jun 12, 2020

Copy link
Copy Markdown
Member

@gasche: I'm happy with @garrigue's response and changes, so I've updated my review to "approve".

@gasche

gasche commented Jun 12, 2020

Copy link
Copy Markdown
Member

@garrigue this can be merged, but you need to fix conflicts first -- in the Changes, and in the generated parser.

@garrigue garrigue force-pushed the injectivity-annotations branch from cc7cbc2 to be9ec2a Compare June 15, 2020 09:06
@garrigue garrigue merged commit 603506a into ocaml:trunk Jun 15, 2020
abbysmal pushed a commit to ocaml-multicore/tezos that referenced this pull request Jun 9, 2021
See ocaml/ocaml#9500 for the introduction of
the breaking change within OCaml.
abbysmal pushed a commit to ocaml-multicore/tezos that referenced this pull request Jun 22, 2021
See ocaml/ocaml#9500 for the introduction of
the breaking change within OCaml.
zshipko pushed a commit to zshipko/tezos that referenced this pull request Oct 7, 2021
See ocaml/ocaml#9500 for the introduction of
the breaking change within OCaml.
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.

4 participants