Commenting style, was RE: [m-dev.] for review: MLDS backend t o do structure reuse and compile time gc

Ralph Becket rbeck at microsoft.com
Wed Oct 11 20:12:54 AEDT 2000


>From Fergus Henderson on 11/10/2000 09:08:07
> That's not my experience.  Code itself is often indented, either due
> to its structure, or because lines often wrap.  So indenting comments
> can and IMHO often does lead to situations where the comments are less
> clearly separated from the code.
> 
> For example, from Tyson's IL back-end diff:
> 
>  |                     % Add a store after the alloc instrs (if necessary)
>  |             { AllocInstrs = list__condense(tree__flatten(
>  |                     tree(AllocComment,
>  |                     tree(AllocInstrsTree, StoreAllocTree)))) },
>  |                     % Add a load before the init instrs (if necessary)
>  |             { InitInstrs = list__condense(tree__flatten(
>  |                     tree(InitComment,
>  |                     tree(LoadTree, tree(InitInstrTree,
StoreInitTree))))) },
> 
> Here the comment is not clearly separated from the code, and even
> worse the indentation makes it look like the second comment belongs
> with the first goal.

I don't think that
>  |             % Add a store after the alloc instrs (if necessary)
>  |             { AllocInstrs = list__condense(tree__flatten(
>  |                     tree(AllocComment,
>  |                     tree(AllocInstrsTree, StoreAllocTree)))) },
>  |             % Add a load before the init instrs (if necessary)
>  |             { InitInstrs = list__condense(tree__flatten(
>  |                     tree(InitComment,
>  |                     tree(LoadTree, tree(InitInstrTree,
StoreInitTree))))) 
is that much clearer.

The problem here is too few newlines, my dear Mozart.  How about this:
>  |                     % Add a store after the alloc instrs (if necessary)
>  |                     %
>  |             { AllocInstrs = list__condense(tree__flatten(
>  |                     tree(AllocComment,
>  |                     tree(AllocInstrsTree, StoreAllocTree)))) },
>  |
>  |                     % Add a load before the init instrs (if necessary)
>  |                     %
>  |             { InitInstrs = list__condense(tree__flatten(
>  |                     tree(InitComment,
>  |                     tree(LoadTree, tree(InitInstrTree,
StoreInitTree))))) 
I think the three extra lines here greatly improve the legibility of
the code (no offense, Tyson).

--
Ralph Becket      |      MSR Cambridge      |      rbeck at microsoft.com 

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list