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

[github patch] introduce Asttypes.arrow_flag to encode labelled arguments (instead of using string manipulations) #6367

Closed
vicuna opened this issue Apr 12, 2014 · 8 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Apr 12, 2014

Original bug ID: 6367
Reporter: @gasche
Assigned to: @garrigue
Status: resolved (set by @garrigue on 2017-08-02T04:13:27Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 4.02.0+dev
Target version: later
Fixed in version: 4.03.0+dev / +beta1
Category: typing
Tags: github, patch
Related to: #6611
Monitored by: jmeber

Bug description

#25

This commit introduce the type arrow_flag in place of string to represent labelled arguments.

Previously, labelled arguments where encoded as a string tagging the left hand-side of an arrow type:
"" is the absence of label, "?ident" for optional arguments, "ident" for labelled arguments.

The arrow_flag variant now brings more structure to function arguments.
The purpose is two fold: offers a proper encoding of labels rather than relying on string processing, and ease further extensions of arrow lhs.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 14, 2014

Comment author: @gasche

Some people wondered about the status of this PR:
in #25, Jacques suggested that he was willing to integrate the change, but probably with a re-implementation rather than a direct merge.

I think this is an important change as it makes further experiments (eg. modular-implicits and #126 ) easier.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 22, 2014

Comment author: @garrigue

Committed in trunk at revision 15737.

type arg_label =
Nolabel
| Labelled of string (* label:T -> ... )
| Optional of string (
?label:T -> ... *)

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 22, 2014

Comment author: @garrigue

Note that camlp4 needs to be modified too...

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 24, 2014

Comment author: @let-def

Minor thing: it seems to me that Typedtree.optional type is made redundant by the stronger typing of labels.

In particular:

  • in Translcore, tests on optional can be replaced by matching on the arg_label.
  • Typecore can be simplified by removing tracking of Optional/Required label
@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 27, 2015

Comment author: @alainfrisch

Shouldn't we take this opportunity to clean up Pexp_fun, which seems a little bit weird, with an "expression option" argument that can be non-None only when the arg_label is Optional. We could introduce another type:

type fun_label =
| Nolabel
| Labelled of string
| Optional of string * expression option

and replace the two first arguments of Pexp_fun/Pcl_fun with one of this type.

@vicuna

This comment has been minimized.

Copy link
Author

commented Mar 27, 2015

Comment author: @alainfrisch

I re-open in order not to forget the previous notes.

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 2, 2015

Comment author: @alainfrisch

it seems to me that Typedtree.optional type is made redundant by the stronger typing of labels.

This has been cleaned up in trunk.

with an "expression option" argument that can be non-None only when the arg_label is Optional

Still to be done.

@vicuna

This comment has been minimized.

Copy link
Author

commented Aug 2, 2017

Comment author: @garrigue

Similar change by 1584803

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.