Changes between Version 22 and Version 23 of Commentary/CodingStyle


Ignore:
Timestamp:
May 2, 2008 8:11:56 AM (7 years ago)
Author:
simonpj
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • Commentary/CodingStyle

    v22 v23  
    77
    88The general rule is to stick to the same coding style as is already used in the file you're editing. If you must make stylistic changes, commit them separately from functional changes, so that someone looking back through the change logs can easily distinguish them.
     9
     10== Comments and commit messages ==
     11
     12Commenting is good but (
     13  * long comments ''interleaved with the code'' can make the code itself incredibly hard to read, and
     14  * long comments ''detached from the code'' are easy to miss when you are editing the code itself, and soon become out of date or even misleading.
     15
     16We have adopted a style that seems to help.  Here's an example:
     17{{{
     18prepareRhs :: SimplEnv -> OutExpr -> SimplM (SimplEnv, OutExpr)
     19-- Adds new floats to the env iff that allows us to return a good RHS
     20prepareRhs env (Cast rhs co)    -- Note [Float coercions]
     21  | (ty1, _ty2) <- coercionKind co      -- Do *not* do this if rhs is unlifted
     22  , not (isUnLiftedType ty1)            -- see Note [Float coercions (unlifted)]
     23  = do  { (env', rhs') <- makeTrivial env rhs
     24        ; return (env', Cast rhs' co) }
     25
     26        ...more equations for prepareRhs....
     27
     28{- Note [Float coercions]
     29~~~~~~~~~~~~~~~~~~~~~~
     30When we find the binding
     31        x = e `cast` co
     32we'd like to transform it to
     33        x' = e
     34        x = x `cast` co         -- A trivial binding
     35There's a chance that e will be a constructor application or function, or something
     36like that, so moving the coerion to the usage site may well cancel the coersions
     37and lead to further optimisation.  Example:
     38        ...more stuff about coercion floating...
     39-}
     40}}}
     41Notice that
     42 * '''Interleaved with the code''' is a short link `Note [Float coercions]`. You can't miss it when you are editing the code, but you can still see the code itself.
     43 * '''Detached from the code''' is the linked comment, starting with the same string `Note [Float coercions]`.  It can be long, and often includes examples.
     44
     45The standard format "`Note [Float coercions]`" serves like URL, to point to an out-of-line comment.  Usually the target is in the same module, but not always.  Sometimes we say
     46{{{
     47    -- See Note [Float coercions] in SpecConstr.lhs
     48}}}
     49
     50Please use this technique.  It's robust, and survives successive changes to the same lines of code.  When you are changing code, it draws attention to non-obvious things you might want to bear in mind.  When you encounter the note itself you can search for the string to find the code that implements the thoughts contained in the comment.
     51
     52Please do not put comments like these in commit messages instead, ''even if the patch is devoted to a single change''.  The information is harder to find in a commit message, and (much worse) there is no explicit indication in the code that there is carefully-written information available about that particular line of code.  Instead, you can refer to the Note from the commit message.
     53
     54(Commit messages can nevertheless contain substantial information, but it is usually of a global nature.  E.g. "This patch modifies 20 files to implement a new form of inlining pragma".)
    955
    1056== Warnings ==