Support empty record declaration #1536

objmagic · 2017-12-18T06:14:10Z

This PR adds support of empty record declaration.

Support empty record declaration can

bring more consistency in the language
solve some practical issues. For example, ocaml-pb can generate proper code for empty protobuf message.

See Mantis #7583 for more details.

Implementation in this PR might be inefficient during type-checking phase because we will search empty record declaration in environment in linear time complexity (instead of other find- methods in env.ml which will use Path.t to do search in a AVL tree). That said, another approach is to allow {} as user-defined constructor as we did in #234

Signed-off-by: Runhang Li <marklrh@gmail.com>

alainfrisch · 2017-12-18T08:14:16Z

bytecomp/translcore.ml

@@ -1284,7 +1284,11 @@ and transl_record loc env fields repres opt_init_expr =
        match repres with
        | Record_regular -> Lconst(Const_block(0, cl))
        | Record_inlined tag -> Lconst(Const_block(tag, cl))
-        | Record_unboxed _ -> Lconst(match cl with [v] -> v | _ -> assert false)
+        | Record_unboxed _ -> Lconst(


What's the point of "unboxing" an empty record type, since the representation ends up being the same as the regular representation?

alainfrisch · 2017-12-18T08:15:24Z

parsing/pprintast.ml

@@ -1397,8 +1398,11 @@ and type_declaration ctxt f x =
    in
    match x.ptype_kind with
    | Ptype_variant xs ->
-        pp f "%t%t@\n%a" intro priv
-          (list ~sep:"@\n" constructor_declaration) xs
+        if xs = [] then


This is about empty variants, not empty records.

alainfrisch · 2017-12-18T08:22:39Z

typing/oprint.ml

-      fprintf ppf " =%a@;<1 2>%a"
-        print_private td.otype_private
-        (print_list print_out_constr (fun ppf -> fprintf ppf "@ | ")) constrs
+      if constrs = [] then


Also for empty variants, not records.

alainfrisch · 2017-12-18T08:23:54Z

typing/typecore.ml

-              None -> newvar (), None
+            | None ->
+                if lid_sexp_list = [] then begin
+                  match Env.find_empty_record env with


This part could be shared with the similar addition type type_pat.

alainfrisch · 2017-12-18T08:26:06Z

parsing/parser.mly

@@ -1707,7 +1707,8 @@ expr_comma_list:
  | expr COMMA expr                             { [$3; $1] }
 ;
 record_expr:
-    simple_expr WITH lbl_expr_list              { (Some $1, $3) }
+                                                { (None, []) }


Empty rules usually have /* empty */ in parser.mly.

objmagic · 2017-12-18T08:28:36Z

@alainfrisch There are some changes about empty variants that I forgot to clean up. I will remove then address your comments next morning when I get up.

murmour · 2017-12-18T19:37:16Z

parsing/parsetree.mli

@@ -407,9 +407,7 @@ and type_declaration =
 and type_kind =
  | Ptype_abstract
  | Ptype_variant of constructor_declaration list
-        (* Invariant: non-empty list *)


The comment related to Ptype_variant shouldn't be removed.

xavierleroy · 2017-12-20T15:56:46Z

At the last developers's meeting, there was consensus to support empty sum types, but empty record types were controversial. Have we really decided to support empty record types? Could we get a decision before spending more time reviewing and polishing this GPR?

objmagic · 2017-12-20T18:20:29Z

@xavierleroy

Hi, Xavier. I understand empty record is controversial but I'd like to open a PR just for some initial discussion. As for empty sum type, I noticed some cases my implementation does not handle. I am going to fix the problem soon.

garrigue · 2017-12-21T00:24:31Z

Ok, then I suppose that we should have a discussion.

Basically, I see at least 2 ways to handle empty records without adding anything to the language:

Have your code generator emit () rather than {}. Doesn't seem too difficult.
We could even make {} an alias for (), but some are going to complain that this is a waste of syntactic resources...
If you want the thing to be nominal, make it a singleton variant type: type t = Empty_t.
This even gives you a way to use the constructor without ambiguity.

Honestly I think that the first solution is sufficient: I cannot really imagine why you would want to make your empty record nominally distinct, as anyway it has no semantical contents.

Regularity is nice, when it is really regular. Adding empty records as records requires modifications to the way we handle records that make me think that they are not regular records.

yallop · 2017-12-28T14:04:41Z

The following code is already valid OCaml (and introduces isomorphic/nominally-distinct types):

    type s = []
    type t = []

so it seems reasonable to allow the essentially equivalent code:

    type s = {}
    type t = {}

But since there are plenty of workarounds and a clear lack of enthusiasm it doesn't seem worth spending much more time on this.

objmagic · 2017-12-31T07:01:51Z

close this PR as it is deemed controversial

objmagic added 17 commits August 10, 2017 17:23

Allow empty variants and records.

589973c

work.

2692840

save.

f653584

Support empty record type.

df0fc73

Allow empty record in pattern

50fca86

Signed-off-by: Runhang Li <marklrh@gmail.com>

Allow empty record in pattern.

d753426

Signed-off-by: Runhang Li <marklrh@gmail.com>

Pretty-print, fix merge error.

811a18c

Signed-off-by: Runhang Li <marklrh@gmail.com>

Use `Atom(0)' instead.

4feb465

Signed-off-by: Runhang Li <marklrh@gmail.com>

Add test cases.

562a70b

Signed-off-by: Runhang Li <marklrh@gmail.com>

Minor fixes.

e765eb3

Signed-off-by: Runhang Li <marklrh@gmail.com>

Add test for undeclared record type.

4dadf1e

Signed-off-by: Runhang Li <marklrh@gmail.com>

no empty variants

e955d1e

Merge branch 'trunk' into empty-record-variants

e8817c4

fix merge error

8262ba0

fix some tests

4942d48

unbox status

7d16112

improve dsource

c5ce8de

alainfrisch reviewed Dec 18, 2017

View reviewed changes

objmagic added 2 commits December 18, 2017 10:04

Write empty in empty rule, remove variants code

26b8e83

Refactor typecore

2d0fae5

murmour reviewed Dec 18, 2017

View reviewed changes

objmagic added 2 commits December 18, 2017 12:40

add back invariant comment.

f8dd923

Add change entry.

07b4a17

objmagic force-pushed the empty-record branch 2 times, most recently from 6ebbda6 to 07b4a17 Compare December 19, 2017 06:26

objmagic mentioned this pull request Dec 23, 2017

[LOWPRI][FUTURE]Support empty variant type, and empty record type. reasonml/reason#1597

Closed

objmagic closed this Dec 31, 2017

EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024

Books page design changes according to Figma (ocaml#1536)

9c2abab

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support empty record declaration #1536

Support empty record declaration #1536

objmagic commented Dec 18, 2017 •

edited

alainfrisch Dec 18, 2017

alainfrisch Dec 18, 2017

alainfrisch Dec 18, 2017

alainfrisch Dec 18, 2017

alainfrisch Dec 18, 2017

objmagic commented Dec 18, 2017

murmour Dec 18, 2017

xavierleroy commented Dec 20, 2017

objmagic commented Dec 20, 2017

garrigue commented Dec 21, 2017

yallop commented Dec 28, 2017

objmagic commented Dec 31, 2017

Support empty record declaration #1536

Support empty record declaration #1536

Conversation

objmagic commented Dec 18, 2017 • edited

alainfrisch Dec 18, 2017

Choose a reason for hiding this comment

alainfrisch Dec 18, 2017

Choose a reason for hiding this comment

alainfrisch Dec 18, 2017

Choose a reason for hiding this comment

alainfrisch Dec 18, 2017

Choose a reason for hiding this comment

alainfrisch Dec 18, 2017

Choose a reason for hiding this comment

objmagic commented Dec 18, 2017

murmour Dec 18, 2017

Choose a reason for hiding this comment

xavierleroy commented Dec 20, 2017

objmagic commented Dec 20, 2017

garrigue commented Dec 21, 2017

yallop commented Dec 28, 2017

objmagic commented Dec 31, 2017

objmagic commented Dec 18, 2017 •

edited