0005936: Weird syntax accepted by OCaml

(0008945)
doligez (manager)
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 (manager)
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 (manager)
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 (manager)
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 (manager)
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 (manager)
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 (manager)
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 (manager)
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.

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

Notes
(0008945) doligez (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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 (manager) 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.