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
Weird syntax accepted by OCaml #5936
Comments
Comment author: @damiendoligez 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. |
Comment author: @alainfrisch
Sorry, I missed that part in the table.
Since I don't see a strong argument to remove it, and this could break existing code, I'm in favor of documenting it. |
Comment author: @damiendoligez
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. |
Comment author: @alainfrisch I won't object to the removal (especially if the same reasoning is applied to #5939). |
Comment author: @damiendoligez I think we should also get rid of this syntax for :: in expressions and patterns: (::) (x, y) |
Comment author: @alainfrisch 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.
If you have some automated tests, could you do the same test for the syntax described #5939? |
Comment author: @lpw25 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. |
Comment author: @damiendoligez 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. |
Comment author: @damiendoligez 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. |
Comment author: @lpw25
I had been hoping at some point to add proper support for (::), including other infix constructors to allow things like: type (_, _) flist = let rec compute x fl : type a b. a -> (a,b) flist -> b = Would this be an acceptable alternative to removing it? |
Comment author: @damiendoligez We need a serious discussion on the developers list before we implement user-defined infix constructors. 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:
I'm also attaching the list of packages used for these tests. |
Comment author: @lpw25
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. |
Comment author: @damiendoligez unary + : no impact 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. |
Comment author: @alainfrisch 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 #5939. |
Comment author: @alainfrisch Damien: another weird (explicitly deprecated) syntax "#c [> `A]" (see #5983). Does your test suite indicate that it could also be removed? |
Comment author: @damiendoligez #c [> `A] : no impact I move that we remove all the "no impact" ones. |
Comment author: @alainfrisch
I suggest we do that as part of the big cleanup/documentation of the Parsetree in the extension_points branch, to avoid merge conflicts. |
Comment author: @johnwhitington Bug #6173 shows another unintended consequence of having () as a constructor. |
Comment author: @alainfrisch
The branch has been merged, so this comment is no longer applicable (cleanup can be done on the trunk). |
Comment author: @damiendoligez About unary +, don't remove it before reading #222 |
Comment author: gerd So we can propagandize now the use of +x as short-hand for (x:int) in expressions? (Just joking. I came recently across the same issue in my query compiler, and it is just there because some standard/expectation says it must be there.) |
Comment author: @paurkedal The unary plus can be used to accentuate the sign, as in let x = ... in + long-expression - long-expression ... + long-expression I'm undecided whether this is better coding style than just leaving the first "+" sign blank. I won't miss the unary plus if it goes, but we shouldn't feel compelled to remove a construct because it's operationally useless. There is already the "|" prefix for type definitions and the ";" suffix in lists. There is precedence for unary plus in C, C#, Java, JavaScript, Python, Perl, and Ruby. In Perl it's also magic [1] (you can safely skip reading that). On the other hand, Rust may reject it [2], and may search certainly missed languages which never considered it. [1] http://perlmaven.com/the-magic-unary-plus |
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
Original bug ID: 5936
Reporter: @alainfrisch
Assigned to: @damiendoligez
Status: assigned (set by @damiendoligez on 2017-02-24T15:35:17Z)
Resolution: open
Priority: low
Severity: minor
Category: lexing and parsing
Related to: #5939 #5983 #6173 #6961
Monitored by: @nojb @Drup @gasche @hcarty @yakobowski
Bug description
I'm surprised that the following syntax is accepted:
let id x = x in
(- id 10)
(and parsed as (- (id 10)).
Same with + instead of -.
File attachments
The text was updated successfully, but these errors were encountered: