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

Weird syntax accepted by OCaml #5936

Closed
vicuna opened this issue Feb 28, 2013 · 23 comments
Closed

Weird syntax accepted by OCaml #5936

vicuna opened this issue Feb 28, 2013 · 23 comments

Comments

@vicuna
Copy link

vicuna commented Feb 28, 2013

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

@vicuna
Copy link
Author

vicuna commented Mar 4, 2013

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.

@vicuna
Copy link
Author

vicuna commented Mar 5, 2013

Comment author: @alainfrisch

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.

@vicuna
Copy link
Author

vicuna commented Mar 6, 2013

Comment author: @damiendoligez

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.

@vicuna
Copy link
Author

vicuna commented Mar 7, 2013

Comment author: @alainfrisch

I won't object to the removal (especially if the same reasoning is applied to #5939).

@vicuna
Copy link
Author

vicuna commented Mar 22, 2013

Comment author: @damiendoligez

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 +).

@vicuna
Copy link
Author

vicuna commented Mar 22, 2013

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.

I haven't found it used anywhere.

If you have some automated tests, could you do the same test for the syntax described #5939?

@vicuna
Copy link
Author

vicuna commented Mar 24, 2013

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.

@vicuna
Copy link
Author

vicuna commented Apr 4, 2013

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.

@vicuna
Copy link
Author

vicuna commented Apr 4, 2013

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.

@vicuna
Copy link
Author

vicuna commented Apr 4, 2013

Comment author: @lpw25

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?

@vicuna
Copy link
Author

vicuna commented Apr 10, 2013

Comment author: @damiendoligez

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.

@vicuna
Copy link
Author

vicuna commented Apr 10, 2013

Comment author: @lpw25

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.

@vicuna
Copy link
Author

vicuna commented Apr 11, 2013

Comment author: @damiendoligez

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.

@vicuna
Copy link
Author

vicuna commented Apr 12, 2013

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.

@vicuna
Copy link
Author

vicuna commented Apr 12, 2013

Comment author: @alainfrisch

Damien: another weird (explicitly deprecated) syntax "#c [> `A]" (see #5983). Does your test suite indicate that it could also be removed?

@vicuna
Copy link
Author

vicuna commented Apr 15, 2013

Comment author: @damiendoligez

#c [> `A] : no impact

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

@vicuna
Copy link
Author

vicuna commented Apr 15, 2013

Comment author: @alainfrisch

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.

@vicuna
Copy link
Author

vicuna commented Sep 19, 2013

Comment author: @johnwhitington

Bug #6173 shows another unintended consequence of having () as a constructor.

@vicuna
Copy link
Author

vicuna commented Sep 20, 2013

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.

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

@vicuna
Copy link
Author

vicuna commented Sep 7, 2015

Comment author: @damiendoligez

About unary +, don't remove it before reading #222

@vicuna
Copy link
Author

vicuna commented Sep 25, 2015

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.)

@vicuna
Copy link
Author

vicuna commented Jul 6, 2016

Comment author: @paurkedal

The unary plus can be used to accentuate the sign, as in if x < 0 then -1 else +1 and it could line up numbers in a multi-line context. Another is to prefix the first line in a multi-line sum:

  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
[2] https://internals.rust-lang.org/t/add-an-optional-prefix-to-numeric-literals/1283

@github-actions
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants