Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006306OCamltypingpublic2014-01-26 21:072017-03-07 13:21
Reportergasche 
Assigned To 
PrioritylowSeveritytweakReproducibilityN/A
StatusclosedResolutionwon't fix 
PlatformOSOS Version
Product Version4.02.0+dev 
Target VersionFixed in Version 
Summary0006306: [style patches] use (@@) to reduce parentheses in typing/predef.ml and asmcomp/cmmgen.ml
DescriptionThe 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 InformationIn 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.
Tagspatch
Attached Filestxt file icon 0001-use-to-avoid-wall-of-parens-in-typing-predef.ml.patch.txt [^] (3,298 bytes) 2014-01-26 21:07 [Show Content]
txt file icon 0002-use-for-sequences-of-bind-in-asmcomp-cmmgen.ml.patch.txt [^] (21,171 bytes) 2014-01-26 21:10 [Show Content]

- Relationships

-  Notes
(0010859)
waern (reporter)
2014-01-29 14:25
edited on: 2014-01-29 14:25

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

(0016870)
shinwell (developer)
2016-12-08 13:29

I think in general I'm against the use of the @@ operator; I find it a detriment to readability.
(0016871)
shinwell (developer)
2016-12-08 13:29

(Does anyone else want to argue for or against this?)
(0016926)
octachron (developer)
2016-12-08 18:45
edited on: 2016-12-08 18:45

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

(0016927)
gasche (developer)
2016-12-08 19:16

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.
(0016929)
shinwell (developer)
2016-12-09 07:51

In which case please only merge the Predef changes. I am firmly against doing this in Cmmgen, and work quite regularly in this file.
(0016930)
shinwell (developer)
2016-12-09 07:55

(Also, FWIW, I think "List.fold_right (@@)" is even worse. Just be simple and explicit for critical code.)
(0017586)
shinwell (developer)
2017-03-07 13:20

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.

- Issue History
Date Modified Username Field Change
2014-01-26 21:07 gasche New Issue
2014-01-26 21:07 gasche File Added: 0001-use-to-avoid-wall-of-parens-in-typing-predef.ml.patch.txt
2014-01-26 21:08 gasche File Added: 0002-use-for-sequences-of-bind-in-asmcomp-cmmgen.ml.patch.txt
2014-01-26 21:09 gasche File Deleted: 0002-use-for-sequences-of-bind-in-asmcomp-cmmgen.ml.patch.txt
2014-01-26 21:10 gasche File Added: 0002-use-for-sequences-of-bind-in-asmcomp-cmmgen.ml.patch.txt
2014-01-26 21:11 gasche Additional Information Updated View Revisions
2014-01-29 14:25 waern Note Added: 0010859
2014-01-29 14:25 waern Note Edited: 0010859 View Revisions
2014-02-19 20:08 doligez Tag Attached: patch
2014-07-16 15:06 doligez Status new => confirmed
2014-07-16 15:06 doligez Target Version => 4.03.0+dev / +beta1
2016-03-23 14:31 doligez Target Version 4.03.0+dev / +beta1 => 4.03.1+dev
2016-12-08 13:29 shinwell Note Added: 0016870
2016-12-08 13:29 shinwell Note Added: 0016871
2016-12-08 18:45 octachron Note Added: 0016926
2016-12-08 18:45 octachron Note Edited: 0016926 View Revisions
2016-12-08 19:16 gasche Note Added: 0016927
2016-12-09 07:51 shinwell Note Added: 0016929
2016-12-09 07:55 shinwell Note Added: 0016930
2017-02-16 14:01 doligez Target Version 4.03.1+dev => undecided
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-02-27 15:08 doligez Category -OCaml general => typing
2017-02-27 15:08 doligez Target Version undecided =>
2017-03-07 13:20 shinwell Note Added: 0017586
2017-03-07 13:21 shinwell Status confirmed => closed
2017-03-07 13:21 shinwell Resolution open => won't fix


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker