Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006611OCaml~DO NOT USE (was: OCaml general)public2014-10-12 23:142016-12-07 11:47
Reporterwhitequark 
Assigned Togarrigue 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version4.02.2+dev / +rc1Fixed in Version4.03.0+dev / +beta1 
Summary0006611: Do something with *predef*.option
DescriptionCurrently, the types of all optional arguments in function signatures must be wrapped in an internal "predef option" type, which can be done with this function:

    let wrap_predef_option typ =
      let predef_option = mknoloc (Ldot (Lident "*predef*", "option")) in
      Typ.constr predef_option [typ]

This is extremely unintuitive. The compiler will normally show types not wrapped in "predef option" as "<hidden>" and complain that the function does not match its signature. In fact, it cost me an hour of debugging before I accidentally looked at Pprintast and noticed an assertion that checks for "predef option".

There are two ways to fix this that I can see.

1) Proper way. Remove the notion of "predef option" from Parsetree entirely. "?" at the start of the label already carries all the necessary information.

2) Workaround. Make Typ.arrow look at label and automatically wrap the type in "predef option" if it starts with "?".
TagsNo tags attached.
Attached Filesdiff file icon patch_6611.diff [^] (3,566 bytes) 2014-10-13 09:49 [Show Content]

- Relationships
related to 0006367resolvedgarrigue [github patch] introduce Asttypes.arrow_flag to encode labelled arguments (instead of using string manipulations) 

-  Notes
(0012362)
frisch (developer)
2014-10-13 09:37
edited on: 2014-10-13 09:50

I'd go for 1), which is technically very simple: just move the code which adds the *predef*.option wrapper from parser.mly to typetexp.ml/typeclass.ml (in the cases for Ptyp_arrow/Pcty_arrow). But this cannot be done lightly, since it would break for instance camlp4/camlp5. One more backward compatible change would be to add the wrapper only if it is not already present in the Parsetree.

I've added a patch which does that (untested), but this seems too risky to go into 4.02.1.

(0012841)
doligez (administrator)
2014-12-17 23:58

The patch looks good, but please add a comment in wrap_option_arg explaining the backward-compatibility workaround.

Also, before closing this PR we will need to push the information to the camlp4 and camlp5 developers.
(0012933)
garrigue (manager)
2014-12-22 10:07

I committed this in trunk, at revision 15738, but without the compatibility provision, since I have just exhauced 6367 which is incompatible at the type level anyway.

- Issue History
Date Modified Username Field Change
2014-10-12 23:14 whitequark New Issue
2014-10-13 09:37 frisch Note Added: 0012362
2014-10-13 09:49 frisch File Added: patch_6611.diff
2014-10-13 09:50 frisch Note Edited: 0012362 View Revisions
2014-10-13 11:35 doligez Target Version => 4.02.2+dev / +rc1
2014-12-17 23:56 doligez Status new => confirmed
2014-12-17 23:58 doligez Note Added: 0012841
2014-12-22 10:07 garrigue Note Added: 0012933
2014-12-22 10:07 garrigue Status confirmed => resolved
2014-12-22 10:07 garrigue Fixed in Version => 4.03.0+dev / +beta1
2014-12-22 10:07 garrigue Resolution open => fixed
2014-12-22 10:07 garrigue Assigned To => garrigue
2014-12-22 10:07 garrigue Relationship added related to 0006367
2016-12-07 11:47 xleroy Status resolved => closed
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-03-03 17:55 doligez Category -OCaml general => -(deprecated) general
2017-03-03 18:01 doligez Category -(deprecated) general => ~deprecated (was: OCaml general)
2017-03-06 17:04 doligez Category ~deprecated (was: OCaml general) => ~DO NOT USE (was: OCaml general)


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker