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

Extend record punning to allow destructuring. #3

Closed
wants to merge 1 commit into from

Conversation

yallop
Copy link
Member

@yallop yallop commented Jan 30, 2014

This patch extends the record punning syntax (b01621e) to allow simultaneous label punning and destructuring. Variables bound using as at the top level of a field pattern are treated as labels. For example, it allows you to write

   fun { { x ; y } as p; q } -> e

which is equivalent to

   fun { p = { x ; y } as p; q } -> e

@mmottl
Copy link
Contributor

mmottl commented Jan 30, 2014

I like the proposal. This syntax change is non-intrusive, intuitive, and would help writing complex pattern matching rules. I sure have some code that could benefit from it.

@samoht
Copy link
Member

samoht commented Jan 30, 2014

Would be nice if this syntax was also able to fix cases such:

type t = { x: int; y: int }
let f { x as t } = t.y

So would be nice if we can rewrite your example to something which looks like:

fun { { x ; y as p } ; q } -> e

@hcarty
Copy link
Member

hcarty commented Jan 30, 2014

I have wanted this or something similar in the past when using nested records. If you change as p to as not_a_field_name would you still get Error: Unbound record field not_a_field_name with proper location information?

@pw374
Copy link

pw374 commented Jan 30, 2014

On 30 Jan 2014, at 14:14 pm, Thomas Gazagnaire notifications@github.com wrote:

Would be nice if this syntax was also able to fix cases such:

type t = { x: int; y: int }
let f { x as t } = t.y
So would be nice if we can rewrite your example to something which looks like:

fun { { x ; y as p } ; q } -> e

I can see that it saves a few characters but it’s not worth it in my opinion.
This syntax feels very odd, because in { x as t } it’s not x that’s as t, but the entire record.
I don’t like it, to me it’s somewhat misleading and unnatural.

@yallop
Copy link
Member Author

yallop commented Jan 30, 2014

@hcarty: yes, location information and error messages are preserved.

# fun { { x ; y } as not_a_field_name; q } -> ();;
Characters 19-35:
  fun { { x ; y } as not_a_field_name; q } -> ();;
                     ^^^^^^^^^^^^^^^^
Error: Unbound record field not_a_field_name

@mmottl
Copy link
Contributor

mmottl commented Jan 30, 2014

@samoht: not bad, but you'd only save having to type an extra pair of parentheses then. The "as" syntax as such already saves you having to duplicate a (potentially long) field name. The "x as t" looks a little strange to me. I think I'd prefer having the binding outside of the enclosing structure as in the first proposal.

@samoht
Copy link
Member

samoht commented Jan 30, 2014

Regarding my proposition, it's exactly the same as for tuples (which already works):

let f (x, _ as t) = snd t

@mmottl
Copy link
Contributor

mmottl commented Jan 30, 2014

@samoht: true, but that's because the tuple syntax, unlike records, allows you to drop parentheses in the first place. The enclosing parentheses here aren't required by the tuple, only by the "as"-binding.

Update: to be precise, the tuple does require the parentheses here, because it's a function definition, but not e.g. in:

let x, _ as t = ...

@pw374
Copy link

pw374 commented Jan 30, 2014

@samoht wrote:

Regarding my proposition, it's exactly the same as for tuples (which already works):

let f (x, _ as t) = snd t

But that’s between parenthesis, not curly braces.
The “as” takes "everything" on its left side, no matter if it’s a tuple or an or-pattern
(as in

# let f = function (`A | `B (_, _) as t) -> t;;
val f : [< `A | `B of 'a * 'b ] -> [> `A | `B of 'a * 'b ] = <fun>

the "as t" takes everything of its left)

@samoht
Copy link
Member

samoht commented Jan 30, 2014

But that’s between parenthesis, not curly braces.

I'm just proposing to have a consistent notation for tuple and records. But I see your point, so I stop arguing :-)

@pw374
Copy link

pw374 commented Jan 30, 2014

I believe I’ve found a better and stronger reason:
in
function { contents = (A | B) as x } -> x
is it the whole record that is as x, or just the value of the field contents? ;-)

@alainfrisch
Copy link
Contributor

Another useful extension of the syntax for record patterns would be:

{ x.a = pa; x.b = pb; x.c; y.t = pt }

equivalent to:

{ x = {a = pa; b = pb; c}; y = {t = pt} };

This would also be useful for record override:

{ r with x.a = ea; x.b = eb; x.c; y.t = et }

equivalent to:

{r with x = {r.x with a = ea; b = eb; c}; y = {r.y with t = et}}

@Kakadu
Copy link
Contributor

Kakadu commented Jan 31, 2014

I'm on @pw374 's side. I don't like

type t = { x: int; y: int }
let f { x as t } = t.y 

It will be great if 2nd line will be parsed as equivalent to let f {y; x as t } = y (t should be a filed of record, not the whole record). Probably idea this change is inspired by camlp4r but I still don't like. If we apply it line let f { y; x as t } = t.y will look strange for me.

@gasche
Copy link
Member

gasche commented Jan 31, 2014

If I can indulge in a small meta note, I think community review works best for proposal that have a good chance of being consensual. As soon as something touches the language definition, the design space explodes and it's very hard to reach agreement (whether in small or larger groups) -- it's one of the reason why maintainers are generally so reluctant with syntax changes. While I don't think Jeremy's initial suggestion was unreasonable, I'll personally prudently avoid trying to obtain an actual decision on this, at least for now.

@pw374
Copy link

pw374 commented Jan 31, 2014

On 31 Jan 2014, at 16:50, @gasche wrote:

If I can indulge in a small meta note, I think community review works best for proposal that have a good chance of being consensual. As soon as something touches the language definition, the design space explodes and it's very hard to reach agreement (whether in small or larger groups) -- it's one of the reason why maintainers are generally so reluctant with syntax changes. While I don't think Jeremy's initial suggestion was unreasonable, I'll personally prudently avoid trying to obtain an actual decision on this, at least for now.

I do like @yallop's initial suggestion. I believe no one on this thread has shown to be against it.

It’s a tiny patch (only 2 reasonable new lines in parsing/parser.mly) and it doesn’t seem to introduce any ambiguity on the syntax.

@mmottl
Copy link
Contributor

mmottl commented Jan 31, 2014

Let me add some support for the first proposal, too. I think it at least deserves a short discussion.

@garrigue
Copy link
Contributor

garrigue commented Feb 1, 2014

I kind of wonder whether this change (the initial proposal) is worthwhile.
Namely, in { { x ; y } as p; q } it is not very intuitive that p is a field name.
Wouldn't it end up obfuscating the code, for a very small change in the number of characters?
This is just a consequence of the fact the alias comes after the aliased pattern in ML, which was probably not a good idea to start with, but we're stuck with that.

@gasche
Copy link
Member

gasche commented Feb 1, 2014

@garrigue : couldn't we fix this by interpreting as as symmetrical pattern intersection, allowing <pattern> as <simple pattern>, or at least an additional <variable> as <simple pattern>?

@damiendoligez
Copy link
Member

@gasche I've always wanted to write x as <pattern> instead of <pattern> as x, so this addition would feel very natural to me. Now adding it to the grammar, that's another can of worms...

yallop pushed a commit to yallop/ocaml that referenced this pull request Mar 18, 2015
bactrian pushed a commit that referenced this pull request Apr 3, 2015
xavierleroy added a commit that referenced this pull request Jul 27, 2015
(Merge of branch cmm-mach-types and PR#115.)

- Register type "Addr" is split into
    . "Val" (well-formed OCaml values, appropriate as GC roots)
    . "Addr" (derived pointers within the heap, must not survive a GC)
- memory_chunk "Word" is split into
    . "Word_val" (OCaml value)
    . "Word_int" (native-sized integer, not a pointer into the heap)

Cmmgen was updated to use Word_val or Word_int as appropriate.

Application #1: fail at compile-time if a derived pointer within the heap
survives a GC point (cf. PR#6484).

Application #2: CSE can do a better job across allocation points
(keep factoring expressions of type Int, Val, Float, but not Addr).

Application #3: slightly fewer roots given to the GC
(e.g. pointers into bigarray data).



git-svn-id: http://caml.inria.fr/svn/ocaml/trunk@16269 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Aug 18, 2015
Fix assertion failure for 'effect E _ ->'
mshinwell referenced this pull request in mshinwell/ocaml Oct 22, 2015
Add Gc.print_stat hook to std_exit.ml
@gasche
Copy link
Member

gasche commented Nov 19, 2015

We discussed this PR at the developer meeting yesterday, and there was opposition (not from me) to including it without a patch allowing to write at least x as <pattern> (or possibly p1 as p2 for general intersection) to use a more natural order.

(We have no guarantees, of course, that a proposal including this richer syntax would be accepted. I would personally support¹ p1 as p2 independently of the present change.)

¹: another thing I would like to have is for the individual patterns in p1 | p2 have variables that do not appear in the other pattern (but only allow to use variables from the intersection in the right-hand-side). Possibly we could restrict this to variables prefixed with _. The idea is that naming things can clarify, even when they are unused.

@yallop
Copy link
Member Author

yallop commented Nov 23, 2015

Thanks for the feedback. I'll investigate supporting the more general syntax.

another think I would like to have is for the individual patterns in p1 | p2 have variables that do not appear in the other pattern (but only allow to use variables from the intersection in the right-hand-side).

I'd like to have that as well. The old patterns Camlp4 extension used to support that behaviour, IIRC. Even more ambitiously, it'd be good to have something similar for GADTs, if they're ever updated to support |-patterns -- i.e. to have p1 | p2 to introduce the constraints that are in the intersection. (However, both these extensions are beyond the scope of this PR, which has already grown rather significantly.)

@damiendoligez damiendoligez added this to the 4.04-or-later milestone Jan 12, 2016
Drup added a commit to Drup/ocaml that referenced this pull request Apr 27, 2016
   fun { { x ; y } as p; q } -> e

is equivalent to

   fun { p = { x ; y } as p; q } -> e
Octachron referenced this pull request in Octachron/ocaml Jul 29, 2019
jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020
jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020
jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 4, 2020
jfehrle added a commit to jfehrle/ocaml that referenced this pull request Aug 5, 2020
gretay-js pushed a commit to gretay-js/ocaml that referenced this pull request May 28, 2021
Use the new file location for emit.mlp files
poechsel pushed a commit to poechsel/ocaml that referenced this pull request Jul 2, 2021
DMaroo added a commit to DMaroo/ocaml that referenced this pull request Dec 16, 2023
    * Code compiles and links successfully
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
add internationalisation support
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Embed HTML files to serve the documentation
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

10 participants