Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005936OCamlOCaml generalpublic2013-02-28 16:592014-07-30 20:54
Reporterfrisch 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusconfirmedResolutionopen 
PlatformOSOS Version
Product Version 
Target Version4.03.0+devFixed in Version 
Summary0005936: Weird syntax accepted by OCaml
DescriptionI'm surprised that the following syntax is accepted:

  let id x = x in
  (- id 10)

(and parsed as (- (id 10)).

Same with + instead of -.
TagsNo tags attached.
Attached Filestxt file icon opam-test-packages.txt [^] (30,182 bytes) 2013-04-10 14:08 [Show Content]
txt file icon opam-test-packages-2.txt [^] (35,764 bytes) 2013-04-11 23:19 [Show Content]

- Relationships
related to 0005939resolveddoligez "fun (type t) when cond -> ..." should be rejected, maybe also "fun p when cond -> ..." 
related to 0005983resolved Bad error message "Unbound class" on deprecated syntax #c [> `A] 

-  Notes
(0008945)
doligez (administrator)
2013-03-05 00:33

What is weird about that? The reference manual clearly states that function application associates more strongly than the unary minus operators.

Unary + is undocumented but works the same. We should decide whether to remove it or document it.
(0008947)
frisch (developer)
2013-03-05 08:58

> The reference manual clearly states that function application associates more strongly than the unary minus operators.

Sorry, I missed that part in the table.

> Unary + is undocumented but works the same. We should decide whether to remove it or document it.

Since I don't see a strong argument to remove it, and this could break existing code, I'm in favor of documenting it.
(0008955)
doligez (administrator)
2013-03-06 23:21

> Since I don't see a strong argument to remove it, and this could break existing code, I'm in favor of documenting it.

Here is a somewhat strong argument to remove it: it is useless. There is no occurrence of it in the OCaml distribution, nor in any of the software listed in testsuite/external/Makefile.
(0008956)
frisch (developer)
2013-03-07 09:17

I won't object to the removal (especially if the same reasoning is applied to 0005939).
(0008999)
doligez (administrator)
2013-03-22 12:30

I think we should also get rid of this syntax for :: in expressions and patterns: (::) (x, y)
It's not documented, and I haven't found it used anywhere.
For the record, I discussed it with Xavier and he agrees (for both :: and unary +).
(0009000)
frisch (developer)
2013-03-22 12:35

FWIW, this syntax was introduced (in patterns and expressions) in this revision:

------------------------------------------------------------------------
r6786 | doligez | 2005-02-16 15:38:02 +0100 (Wed, 16 Feb 2005) | 2 lines

alias (::)(a,b) pour (a::b) (suggestion Monniaux)
------------------------------------------------------------------------

Maybe one should check with David that he doesn't have code depending on this feature.

> I haven't found it used anywhere.

If you have some automated tests, could you do the same test for the syntax described 0005939?
(0009003)
lpw25 (developer)
2013-03-24 10:53
edited on: 2013-03-24 10:54

I think that syntax was introduced to make automatic code generation easier. It means that the arguments to a constructor can always be printed the same way.

(0009030)
doligez (administrator)
2013-04-04 11:34

Re: automatic code generation, you need to special-case :: anyway (to print it as (::)), and then why not also special-case the printing of the arguments. For example that's what Coq extraction does. Anyway, if we don't remove it we should document it, and that when you see why I don't like it: it looks like the (+) notation for infix functions, but actually it doesn't work in the same way: you are not allowed to write Module.(::). I'll contact David.

Re: automated tests, I'm currently installing opam with as many packages as I can. When I'm done, I'll do changes to the compiler and tell opam to recompile everything to see what happens. I'll report here.
(0009031)
doligez (administrator)
2013-04-04 11:42

Another syntax wart that I'd like to get rid of: the ability to use "()" "::" "true" and "false" as constructors in a type definition. So far I've found only one use in the wild and it's clearly a mistake.
(0009032)
lpw25 (developer)
2013-04-04 12:02

> and that when you see why I don't like it: it looks like the (+) notation for infix functions, but actually it doesn't work in the same way

I had been hoping at some point to add proper support for (::), including other infix constructors to allow things like:

type (_, _) flist =
    FNil : ('a, 'a) flist
  | (::>) : ('a -> 'b) * ('b, 'c) flist -> ('a, 'c) flist

let rec compute x fl : type a b. a -> (a,b) flist -> b =
  match fl with
  | FNil -> x
  | f ::> rest -> compute (f x) rest

Would this be an acceptable alternative to removing it?
(0009062)
doligez (administrator)
2013-04-10 14:03
edited on: 2013-04-10 14:07

We need a serious discussion on the developers list before we implement user-defined infix constructors.
In the meantime, I've contacted David and he bounced me to Pierre Letouzey because that feature was used for Coq extraction. Pierre says he likes the feature but doesn't use it anymore, so he won't object if we remove it. I don't know what to do.

On the testing front, I've configured opam to my liking (with 327 packages and reproducible behavior), and I'll start testing for uses of the following features:
- unary +
- fun x when e -> e
- (::)
- (), ::, true, false as constructors

I'm also attaching the list of packages used for these tests.

(0009066)
lpw25 (developer)
2013-04-10 14:59

> On the testing front, I've configured opam to my liking (with 327 packages and reproducible behavior), and I'll start testing for uses of the following features

That seems like the most sensible thing to do.

I don't actually object to removing (::). If I persuade the developers list to add support for user-defined infix constructors then (::) can be re-added later.
(0009083)
doligez (administrator)
2013-04-11 13:55
edited on: 2013-04-12 13:09

unary + : no impact
fun ... when : no impact
(::) : no impact
::, true, false : no impact
() : breaks gettext

I have updated the list of test packages (I removed mirage-net and mirage-www).

About () and gettext, it's clearly a mistake in the gettext source, but it works because of this feature of the compiler. I have reported it upstream with a suggestion to fix it, but I don't think we should remove the feature before a new version of gettext is released and packaged.

(0009086)
frisch (developer)
2013-04-12 14:13

Damien: do you see any reason not to remove the other forms? I'd like to get rid of "fun ... when" especially, in order to fix in a simple way 0005939.
(0009087)
frisch (developer)
2013-04-12 19:22

Damien: another weird (explicitly deprecated) syntax "#c [> `A]" (see 0005983). Does your test suite indicate that it could also be removed?
(0009115)
doligez (administrator)
2013-04-15 18:44

#c [> `A] : no impact

I move that we remove all the "no impact" ones.
(0009117)
frisch (developer)
2013-04-15 19:16

> I move that we remove all the "no impact" ones.

I suggest we do that as part of the big cleanup/documentation of the Parsetree in the extension_points branch, to avoid merge conflicts.
(0010380)
johnwhitington (reporter)
2013-09-19 15:13

Bug 0006173 shows another unintended consequence of having () as a constructor.
(0010384)
frisch (developer)
2013-09-20 18:43

> I suggest we do that as part of the big cleanup/documentation of the Parsetree in the extension_points branch, to avoid merge conflicts.

The branch has been merged, so this comment is no longer applicable (cleanup can be done on the trunk).

- Issue History
Date Modified Username Field Change
2013-02-28 16:59 frisch New Issue
2013-03-05 00:33 doligez Note Added: 0008945
2013-03-05 00:33 doligez Status new => feedback
2013-03-05 08:58 frisch Note Added: 0008947
2013-03-05 08:58 frisch Status feedback => new
2013-03-06 23:21 doligez Note Added: 0008955
2013-03-06 23:22 doligez Status new => feedback
2013-03-07 09:17 frisch Note Added: 0008956
2013-03-07 09:17 frisch Status feedback => new
2013-03-22 12:30 doligez Note Added: 0008999
2013-03-22 12:30 doligez Status new => confirmed
2013-03-22 12:35 frisch Note Added: 0009000
2013-03-24 10:53 lpw25 Note Added: 0009003
2013-03-24 10:54 lpw25 Note Edited: 0009003 View Revisions
2013-04-04 11:34 doligez Note Added: 0009030
2013-04-04 11:42 doligez Note Added: 0009031
2013-04-04 12:02 lpw25 Note Added: 0009032
2013-04-10 14:03 doligez Note Added: 0009062
2013-04-10 14:07 doligez Note Edited: 0009062 View Revisions
2013-04-10 14:08 doligez File Added: opam-test-packages.txt
2013-04-10 14:59 lpw25 Note Added: 0009066
2013-04-10 15:50 doligez Note Added: 0009068
2013-04-10 15:59 doligez Note Deleted: 0009068
2013-04-11 13:55 doligez Note Added: 0009083
2013-04-11 19:37 doligez Note Edited: 0009083 View Revisions
2013-04-11 23:17 doligez Note Edited: 0009083 View Revisions
2013-04-11 23:18 doligez Note Edited: 0009083 View Revisions
2013-04-11 23:19 doligez File Added: opam-test-packages-2.txt
2013-04-12 11:27 doligez Note Edited: 0009083 View Revisions
2013-04-12 13:09 doligez Note Edited: 0009083 View Revisions
2013-04-12 13:49 frisch Relationship added related to 0005939
2013-04-12 14:13 frisch Note Added: 0009086
2013-04-12 19:22 frisch Note Added: 0009087
2013-04-15 12:06 doligez Relationship added related to 0005983
2013-04-15 18:44 doligez Note Added: 0009115
2013-04-15 19:16 frisch Note Added: 0009117
2013-07-12 09:38 doligez Target Version => 4.02.0+dev
2013-07-12 18:15 doligez Target Version 4.02.0+dev => 4.01.1+dev
2013-09-19 15:13 johnwhitington Note Added: 0010380
2013-09-20 18:43 frisch Note Added: 0010384
2014-05-25 20:20 doligez Target Version 4.01.1+dev => 4.02.0+dev
2014-07-30 20:54 doligez Target Version 4.02.0+dev => 4.03.0+dev


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker