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

[style patches] use (@@) to reduce parentheses in typing/predef.ml and asmcomp/cmmgen.ml #6306

Closed
vicuna opened this issue Jan 26, 2014 · 8 comments

Comments

@vicuna
Copy link

vicuna commented Jan 26, 2014

Original bug ID: 6306
Reporter: @gasche
Status: closed (set by @mshinwell on 2017-03-07T12:21:04Z)
Resolution: won't fix
Priority: low
Severity: tweak
Version: 4.02.0+dev
Category: typing
Tags: patch

Bug description

The two attached patches make use of the new-in-4.01 standard operator (@@) to avoid wall-of-closing-parenthesis in predef.ml and cmmgen.ml.

The patch in predef would simplify maintenance as it makes adding or removing predefined types (marginally) simpler -- I thought of it while reviewing Benoît's GADT patches.

The patch in cmmgen harmonizes the use of the monadic-like "bind" operator, and also remove some extraneous parentheses. I think it is of (even) lesser importance.

Additional information

In cmmgen.ml I used (@@) only bind was used more than once in a sequence. If someone thinks it's better to consistently use (@@) everywhere, I'd be happy to make this additional change.

File attachments

@vicuna
Copy link
Author

vicuna commented Jan 29, 2014

Comment author: waern

If you use an infix operator like (>>=) instead of bind, you don't need any parenthesis and you don't need (@@).

@vicuna
Copy link
Author

vicuna commented Dec 8, 2016

Comment author: @mshinwell

I think in general I'm against the use of the @@ operator; I find it a detriment to readability.

@vicuna
Copy link
Author

vicuna commented Dec 8, 2016

Comment author: @mshinwell

(Does anyone else want to argue for or against this?)

@vicuna
Copy link
Author

vicuna commented Dec 8, 2016

Comment author: @Octachron

I personally consider a wall of 30 parentheses a bigger obstacle to readability than (@@). A potential compromise – at least for typing/predef.ml – might be to use a simple list and a List.fold_right (@@) rather than sequence of (@@).

@vicuna
Copy link
Author

vicuna commented Dec 8, 2016

Comment author: @gasche

I read the two last messages from my mailbox without much context or memory of the report, and I thought "yeah I agree that (@@) is actually nicer for maintainability in deep-paren-nesting scenarios". I now realize that I wrote the initial patches, duh. I support both changes and I may decide to merge them.

@vicuna
Copy link
Author

vicuna commented Dec 9, 2016

Comment author: @mshinwell

In which case please only merge the Predef changes. I am firmly against doing this in Cmmgen, and work quite regularly in this file.

@vicuna
Copy link
Author

vicuna commented Dec 9, 2016

Comment author: @mshinwell

(Also, FWIW, I think "List.fold_right (@@)" is even worse. Just be simple and explicit for critical code.)

@vicuna
Copy link
Author

vicuna commented Mar 7, 2017

Comment author: @mshinwell

Since there's been no further activity on this PR for three months, I'm afraid I'm going to close it. We have many more important ones to look at.

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

1 participant