FIX #1861: floating-point constants for infinity and NaN in via-C
[ghc-hetmet.git] / docs / coding-style.html
index ba93509..8fbb276 100644 (file)
@@ -1,22 +1,23 @@
 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
 <HTML>
 <HEAD>
-   <TITLE>Access To The GHC CVS Repository</TITLE>
-   <META NAME="GENERATOR" CONTENT="Mozilla/3.04Gold (X11; I; SunOS 5.5.1 i86pc) [Netscape]">
+   <meta http-equiv="content-type" content="text/html;charset=iso-8859-1">
+   <TITLE>Style Guidelines for fptools</TITLE>
 </HEAD>
 <BODY>
 
-<H1>Coding suggestions for GHC/Hugs related code</h1>
+<H1>Style Guidelines for fptools</h1>
 
 <h2>Comments</h2>
 
-NB These are just suggestions.  They're not set in stone.  Some of
-them are probably misguided.  If you disagree with them, feel free to 
-modify this document (and make your commit message reasonably informative)
-or mail someone (eg <a href="fp-cvs-fptools@dcs.gla.ac.uk">The FP-CVS mailing list</a> or
-<a
-href="mailto:reid-alastair@cs.yale.edu">reid-alastair@cs.yale.edu</a>).
+<p>These coding style guidelines are mainly intended for use in
+<tt>ghc/rts</tt> and <tt>ghc/includes</tt>.
 
+<p>NB These are just suggestions.  They're not set in stone.  Some of
+them are probably misguided.  If you disagree with them, feel free to
+modify this document (and make your commit message reasonably
+informative) or mail someone (eg. <a
+href="glasgow-haskell-users@haskell.org">The GHC mailing list</a>)
 
 <h2>References</h2>
 
@@ -24,68 +25,90 @@ If you haven't read them already, you might like to check the following.
 Where they conflict with our suggestions, they're probably right.
 
 <ul>
+
 <li>
+The C99 standard.  One reasonable reference is <a
+href="http://home.tiscalinet.ch/t_wolf/tw/c/c9x_changes.html">here</a>.
+
+<p><li>
 Writing Solid Code, Microsoft Press.  (Highly recommended.  Possibly
-the only Microsoft Press book that's worth reading.  SimonPJ has a
-copy.)
+the only Microsoft Press book that's worth reading.)
 
-<li>
-Autoconf documentation (which doesn't seem to be on the web).
+<p><li>
+Autoconf documentation.
 See also <a href="http://peti.gmd.de/autoconf-archive/">The autoconf macro archive</a> and 
 <a href="http://www.cyclic.com/cyclic-pages/autoconf.html">Cyclic Software's description</a>
 
-<li>
-<a href="http://www.cs.umd.edu/users/cml/cstyle/indhill-cstyle.html">Indian Hill C Style and Coding Standards</a>.
+<p><li> <a
+href="http://www.cs.umd.edu/users/cml/cstyle/indhill-cstyle.html">Indian
+Hill C Style and Coding Standards</a>.
 
-<li>
+<p><li>
 <a href="http://www.cs.umd.edu/users/cml/cstyle/">A list of C programming style links</a>
 
-<li>
+<p><li>
 <a href="http://www.lysator.liu.se/c/c-www.html">A very large list of C programming links</a>
 
-<li>
+<p><li>
 <a href="http://www.geek-girl.com/unix.html">A list of Unix programming links</a>
 
-
 </ul>
 
 
 <h2>Portability issues</h2>
 
 <ul>
-<li>
-We use ANSI C with some extensions.  In particular, we use:
+<li> We try to stick to C99 where possible.  We use the following
+C99 features relative to C89, some of which were previously GCC
+extensions (possibly with different syntax):
+
 <ul>
-<li>enum
-<li>ANSI style prototypes
-<li>#elsif, #error, #warning, ## and other cpp features
+<li>Variable length arrays as the last field of a struct.  GCC has
+a similar extension, but the syntax is slightly different: in GCC you
+would declare the array as <tt>arr[0]</tt>, whereas in C99 it is
+declared as <tt>arr[]</tt>.
+
+<p><li>Inline annotations on functions (see later)
+
+<p><li>Labeled elements in initialisers.  Again, GCC has a slightly
+different syntax from C99 here, and we stick with the GCC syntax until
+GCC implements the C99 proposal.
+
+<p><li>C++-style comments.  These are part of the C99 standard, and we
+prefer to use them whenever possible.
 </ul>
 
-<li>
-We use the following gcc extensions (see gcc documentation):
+<p>In addition we use ANSI-C-style function declarations and
+prototypes exclusively.  Every function should have a prototype;
+static function prototypes may be placed near the top of the file in
+which they are declared, and external prototypes are usually placed in
+a header file with the same basename as the source file (although there
+are exceptions to this rule, particularly when several source files
+together implement a subsystem which is described by a single external
+header file).
+
+<p><li>We use the following GCC extensions, but surround them with
+<tt>#ifdef __GNUC__</tt>:
+
 <ul>
-<li>zero length arrays (useful as the last field of a struct)
-<li>inline annotations on functions (see later)
-<li>Labeled elements in initialisers
-<li>Function attributes (mostly just no_return)
-<li>Macro varargs (actually, we don't use them yet but I'm very tempted)
-<li>Alastair really likes to use C++ style comments - but
-    he'll probably regret it later.
-<li>other stuff I've forgotten about...
+<li>Function attributes (mostly just <code>no_return</code> and
+<code>unused</code>)
+<li>Inline assembly.
 </ul>
 
-Some of these gcc/ANSI features could be avoided (for example, we
-could use __inline__ instead of inline or use the usual PROTO((...))
-trick in function prototypes) - but some of them can't be avoided
-so we don't try with the others.
-
-<li>
+<p><li>
 char can be signed or unsigned - always say which you mean
 
-<li>
-Some architectures have memory alignment constraints.
-Others don't have any constraints but go faster if you align things.
-These macros tell you which alignment to use
+<p><li>Our POSIX policy: try to write code that only uses POSIX (IEEE
+Std 1003.1) interfaces and APIs.  We used to define
+<code>POSIX_SOURCE</code> by default, but found that this caused more
+problems than it solved, so now we require any code that is
+POSIX-compliant to explicitly say so by having <code>#include
+"PosixSource.h"</code> at the top.  Try to do this whenever possible.
+
+<p><li> Some architectures have memory alignment constraints.  Others
+don't have any constraints but go faster if you align things.  These
+macros (from <tt>config.h</tt>) tell you which alignment to use
 
 <pre>
   /* minimum alignment of unsigned int */
@@ -101,50 +124,39 @@ These macros tell you which alignment to use
   #define ALIGNMENT_DOUBLE 4
 </pre>
 
-<li>
-Use StgInt, StgWord and StgPtr when reading/writing ints and ptrs to
-the stack or heap.  Note that, by definition, StgInt, StgWord and
-StgPtr are the same size and have the same alignment constraints
-even if sizeof(int) != sizeof(ptr) on that platform.
+<p><li> Use <tt>StgInt</tt>, <tt>StgWord</tt> and <tt>StgPtr</tt> when
+reading/writing ints and ptrs to the stack or heap.  Note that, by
+definition, <tt>StgInt</tt>, <tt>StgWord</tt> and <tt>StgPtr</tt> are
+the same size and have the same alignment constraints even if
+<code>sizeof(int) != sizeof(ptr)</code> on that platform.
 
-<li>
-Use StgInt8, StgInt16, etc when you need a certain minimum number of
-bits in a type.  Use int and nat when there's no particular
-constraint.  ANSI C only guarantees that ints are at least 16 bits but
-within GHC we assume they are 32 bits.  (I'm not sure if this is a
-good idea - ADR)
+<p><li> Use <tt>StgInt8</tt>, <tt>StgInt16</tt>, etc when you need a
+certain minimum number of bits in a type.  Use <tt>int</tt> and
+<tt>nat</tt> when there's no particular constraint.  ANSI C only
+guarantees that ints are at least 16 bits but within GHC we assume
+they are 32 bits.
 
-<li>
-Use StgFloat and StgDouble for floating point values which will go
-on/have come from the stack or heap.  Note that StgFloat may be the
-same as StgDouble on some architectures (eg Alphas) and that it might
-occupy many StgWords.
+<p><li> Use <tt>StgFloat</tt> and <tt>StgDouble</tt> for floating
+point values which will go on/have come from the stack or heap.  Note
+that <tt>StgDouble</tt> may occupy more than one <tt>StgWord</tt>, but
+it will always be a whole number multiple.
 
 <p>
-Use PK_FLT(addr), PK_DBL(addr) to read StgFloat and
-StgDouble values from the stack/heap, and ASSIGN_FLT(val,addr)/
-ASSIGN_DBL(val,addr) to assign StgFloat/StgDouble values to heap/stack
-locations.  These macros take care of alignment restrictions.
+Use <code>PK_FLT(addr)</code>, <code>PK_DBL(addr)</code> to read
+<tt>StgFloat</tt> and <tt>StgDouble</tt> values from the stack/heap,
+and <code>ASSIGN_FLT(val,addr)</code> /
+<code>ASSIGN_DBL(val,addr)</code> to assign StgFloat/StgDouble values
+to heap/stack locations.  These macros take care of alignment
+restrictions.
 
 <p>
-Heap/Stack locations are StgWord aligned; the alignment requirements
-of an StgDouble may be more than that of StgWord, but we don't pad
-misaligned StgDoubles (doing so would be too much hassle).  
+Heap/Stack locations are always <tt>StgWord</tt> aligned; the
+alignment requirements of an <tt>StgDouble</tt> may be more than that
+of <tt>StgWord</tt>, but we don't pad misaligned <tt>StgDoubles</tt>
+because doing so would be too much hassle (see <code>PK_DBL</code> &
+co above).
 
-<p>
-Doing a direct assignment/read of an StgDouble to/from a mis-aligned
-location may not work, so we use the ASSIGN_DBL(,)/PK_DBL() macro,
-which goes via a temporary.  
-
-<p>
-Problem: if the architecture allows mis-aligned accesses, but prefers
-aligned accesses, these macros just add an extra level of indirection.
-We need to distinguish between an architecture that allows mis-aligned
-accesses and one that just imposes a performance penalty (this is most
-of them).  Perhaps have PREFERRED_ALIGNMENT and REQUIRED_ALIGMENT
-configurations?
-
-<li>
+<p><li>
 Avoid conditional code like this:
 
 <pre>
@@ -153,7 +165,7 @@ Avoid conditional code like this:
   #endif
 </pre>
 
-Instead, add an appropriate test to the configure.in script and use
+Instead, add an appropriate test to the configure.ac script and use
 the result of that test instead. 
 
 <pre>
@@ -162,7 +174,7 @@ the result of that test instead.
   #endif
 </pre>
 
-The problem is that things change from one version of an OS to another
+<p>The problem is that things change from one version of an OS to another
 - things get added, things get deleted, things get broken, some things
 are optional extras.  Using "feature tests" instead of "system tests"
 makes things a lot less brittle.  Things also tend to get documented
@@ -180,15 +192,54 @@ than trying to find a bug which only shows up when running GHC on
 itself and doesn't manifest itself until 10 seconds after the actual
 cause of the problem.
 
+<p>We put all our debugging code inside <tt>#ifdef DEBUG</tt>.  The
+general policy is we don't ship code with debugging checks and
+assertions in it, but we do run with those checks in place when
+developing and testing.  Anything inside <tt>#ifdef DEBUG</tt> should
+not slow down the code by more than a factor of 2.
+
+<p>We also have more expensive "sanity checking" code for hardcore
+debugging - this can slow down the code by a large factor, but is only
+enabled on demand by a command-line flag.  General sanity checking in
+the RTS is currently enabled with the <tt>-DS</tt> RTS flag.
+
+<p>There are a number of RTS flags which control debugging output and
+sanity checking in various parts of the system when <tt>DEBUG</tt> is
+defined.  For example, to get the scheduler to be verbose about what
+it is doing, you would say <tt>+RTS -Ds -RTS</tt>.  See
+<tt>includes/RtsFlags.h</tt> and <tt>rts/RtsFlags.c</tt> for the full
+set of debugging flags.  To check one of these flags in the code,
+write:
+
+<pre>
+  IF_DEBUG(gc, fprintf(stderr, "..."));
+</pre>
+
+would check the <tt>gc</tt> flag before generating the output (and the
+code is removed altogether if <tt>DEBUG</tt> is not defined).
+
+<p>All debugging output should go to <tt>stderr</tt>.
+
 <p>
-The ideas in this section are mostly aimed at this issue:
+Particular guidelines for writing robust code:
 
 <ul>
 <li>
 Use assertions.  Use lots of assertions.  If you write a comment
 that says "takes a +ve number" add an assertion.  If you're casting
 an int to a nat, add an assertion.  If you're casting an int to a char,
-add an assertion.
+add an assertion.  We use the <tt>ASSERT</tt> macro for writing
+assertions; it goes away when <tt>DEBUG</tt> is not defined.
+
+<li>
+Write special debugging code to check the integrity of your data structures.
+(Most of the runtime checking code is in <tt>rts/Sanity.c</tt>)
+Add extra assertions which call this code at the start and end of any
+code that operates on your data structures.
+
+<li>
+When you find a hard-to-spot bug, try to think of some assertions,
+sanity checks or whatever that would have made the bug easier to find.
 
 <li>
 When defining an enumeration, it's a good idea not to use 0 for normal
@@ -204,7 +255,15 @@ typedef enum
 
     ...
 </pre>
-<li> Use #warning or #error whenever you write a piece of incomplete/broken code.
+
+<p><li> Use <tt>#warning</tt> or <tt>#error</tt> whenever you write a
+piece of incomplete/broken code.
+
+<p><li> When testing, try to make infrequent things happen often.
+     For example, make a context switch/gc/etc happen every time a 
+     context switch/gc/etc can happen.  The system will run like a 
+     pig but it'll catch a lot of bugs.
+
 </ul>
 
 <h2>Syntactic details</h2>
@@ -222,6 +281,7 @@ In particular:
 Put braces round the body of for loops, while loops, if statements, etc.
 even if they "aren't needed" because it's really hard to find the resulting
 bug if you mess up.  Indent them any way you like but put them in there!
+</ul>
 
 <li>
 When defining a macro, always put parens round args - just in case.
@@ -234,73 +294,9 @@ instead of
   #define add(x,y) x+y
 </pre>
 
-<li>
-Don't define macros that expand to a list of statements.  
-You could just use braces as in:
-
-<pre>
-  #define ASSIGN_CC_ID(ccID)              \
-        {                                 \
-        ccID = CC_ID;                     \
-        CC_ID++;                          \
-        }
-</pre>
-
-but it's better to use the "do { ... } while (0)" trick instead:
-
-<pre>
-  #define ASSIGN_CC_ID(ccID)              \
-        do {                              \
-        ccID = CC_ID;                     \
-        CC_ID++;                          \
-        } while(0)
-</pre>
-
-The following explanation comes from 
-<a href="http://www.cs.umd.edu/users/cml/cstyle/code-std-disc.txt">The Usenet C programming FAQ</a>
-<pre>
-10.4:   What's the best way to write a multi-statement macro?
-
-A:      The usual goal is to write a macro that can be invoked as if it
-        were a statement consisting of a single function call.  This
-        means that the "caller" will be supplying the final semicolon,
-        so the macro body should not.  The macro body cannot therefore
-        be a simple brace-enclosed compound statement, because syntax
-        errors would result if it were invoked (apparently as a single
-        statement, but with a resultant extra semicolon) as the if
-        branch of an if/else statement with an explicit else clause.
-
-        The traditional solution, therefore, is to use
-
-                #define MACRO(arg1, arg2) do {  \
-                        /* declarations */      \
-                        stmt1;                  \
-                        stmt2;                  \
-                        /* ... */               \
-                        } while(0)      /* (no trailing ; ) */
-
-        When the caller appends a semicolon, this expansion becomes a
-        single statement regardless of context.  (An optimizing compiler
-        will remove any "dead" tests or branches on the constant
-        condition 0, although lint may complain.)
-
-        If all of the statements in the intended macro are simple
-        expressions, with no declarations or loops, another technique is
-        to write a single, parenthesized expression using one or more
-        comma operators.  (For an example, see the first DEBUG() macro
-        in question 10.26.)  This technique also allows a value to be
-        "returned."
-
-        References: H&S Sec. 3.3.2 p. 45; CT&P Sec. 6.3 pp. 82-3.
-</pre>
-
-<li>
-Don't even write macros that expand to 0 statements - they can mess you 
-up as well.  Use the doNothing macro instead.
-<pre>
-  #define doNothing() do { } while (0)
-</pre>
-</ul>
+<li> Don't declare and initialize variables at the same time.
+Separating the declaration and initialization takes more lines, but
+make the code clearer.
 
 <li>
 Use inline functions instead of macros if possible - they're a lot
@@ -315,6 +311,10 @@ of side effects, evaluation order, multiple evaluation, etc.
 <li> Inline functions have call-by-value semantics whereas macros
   are call-by-name.  You can be bitten by duplicated computation
   if you aren't careful.
+
+<li> You can use inline functions from inside gdb if you compile with
+  -O0 or -fkeep-inline-functions.  If you use macros, you'd better
+  know what they expand to.
 </ul>
 
 However, note that macros can serve as both l-values and r-values and
@@ -378,11 +378,53 @@ If any uses of the function remain, they will refer to the single copy
 in the library.
 </pre>
 
+<p><li>
+Don't define macros that expand to a list of statements.  
+You could just use braces as in:
+
+<pre>
+  #define ASSIGN_CC_ID(ccID)              \
+        {                                 \
+        ccID = CC_ID;                     \
+        CC_ID++;                          \
+        }
+</pre>
+
+(but it's usually better to use an inline function instead - see above).
+
+<p><li>
+Don't even write macros that expand to 0 statements - they can mess you 
+up as well.  Use the doNothing macro instead.
+<pre>
+  #define doNothing() do { } while (0)
+</pre>
+
 <li>
-Try to use ANSI C's enum feature when defining lists of constants of the
-same type.  You'll notice that gdb works better when you do this.
-For example:
+This code
+<pre>
+int* p, q;
+</pre>
+looks like it declares two pointers but, in fact, only p is a pointer.
+It's safer to write this:
+<pre>
+int* p;
+int* q;
+</pre>
+You could also write this:
+<pre>
+int *p, *q;
+</pre>
+but it is preferrable to split the declarations.
+
+<li>
+Try to use ANSI C's enum feature when defining lists of constants of
+the same type.  Among other benefits, you'll notice that gdb uses the
+name instead of its (usually inscrutable) number when printing values
+with enum types and gdb will let you use the name in expressions you
+type.  
 
+<p>
+Examples:
 <pre>
     typedef enum { /* N.B. Used as indexes into arrays */
      NO_HEAP_PROFILING,                
@@ -424,44 +466,35 @@ instead of
     # define TYPEchar  'Y'
     # define TIMEchar  'T'
 </pre>
-ToDo: at the time of writing, we still use the former.
 
-<li>
-Alastair likes to use stgCast instead of C syntax.  He claims
-it's easier to write and easier to grep for. YMMV.
-<pre>
-#define stgCast(ty,e) ((ty)(e))
-</pre>
+<li> Please keep to 80 columns: the line has to be drawn somewhere,
+and by keeping it to 80 columns we can ensure that code looks OK on
+everyone's screen.  Long lines are hard to read, and a sign that the
+code needs to be restructured anyway.
 
-<li> We don't care too much about your indentation style but, if
-you're modifying a function, please try to use the same style as the
-rest of the function (or file).  
+<li> When commenting out large chunks of code, use <code>#ifdef 0
+... #endif</code> rather than <code>/* ... */</code> because C doesn't
+have nested comments.
+
+<li>When declaring a typedef for a struct, give the struct a name
+as well, so that other headers can forward-reference the struct name
+and it becomes possible to have opaque pointers to the struct.  Our
+convention is to name the struct the same as the typedef, but add a
+leading underscore.  For example:
 
-<p>
-On which note:
-Hugs related pieces of code often start with the line:
-<pre>
-  /* -*- mode: hugs-c; -*- */
-</pre>
-which helps Emacs mimic the indentation style used by Mark P Jones
-within Hugs.  Add this to your .emacs file.
 <pre>
-  (defun hugs-c-mode ()
-    "C mode with adjusted defaults for use with Hugs (based on linux-c-mode)"
-    (interactive)
-    (c-mode)
-    (setq c-basic-offset 4)
-    (setq indent-tabs-mode nil) ; don't use tabs to indent
-    (setq c-recognize-knr-r nil)  ; no K&R here - don't pay the price
-    ; no: (setq tab-width 4)
-
-    (c-set-offset 'knr-argdecl-intro    0)
-    (c-set-offset 'case-label           0)
-    (c-set-offset 'statement-case-intro '++)
-    (c-set-offset 'statement-case-open  '+)
-    )
+  typedef struct _Foo {
+    ...
+  } Foo;
 </pre>
 
+<li>Do not use <tt>!</tt> instead of explicit comparison against
+<tt>NULL</tt> or <tt>'\0'</tt>;  the latter is much clearer.
+
+<li> We don't care too much about your indentation style but, if
+you're modifying a function, please try to use the same style as the
+rest of the function (or file).  If you're writing new code, a
+tab width of 4 is preferred.
 
 </ul>
 
@@ -473,12 +506,37 @@ Don't be tempted to reindent or reorganise large chunks of code - it
 generates large diffs in which it's hard to see whether anything else
 was changed.
 <p>
-If you must reindent or reorganise, don't do anything else in that commit
-and give advance warning that you're about to do it in case anyone else
-is changing that file.
+If you must reindent or reorganise, don't include any functional
+changes that commit and give advance warning that you're about to do
+it in case anyone else is changing that file.
 </ul>
 
 
+<h2>Commandline arguments</h2>
+
+A program in fptools should try follow the following rules for
+commandline arguments:
+
+<ul>
+<li> The <code>-v</code> and <code>--verbose</code> options should be
+used to generate verbose output (intended for the user).
+
+<li> The <code>-d</code> and <code>--debug</code> options should be
+used to generate debugging output (intended for the developer).
+
+<li> The <code>-?</code> and <code>--help</code> options should be used
+to display usage information on stdout. The program should exit
+successfully afterwards.
+
+<li> The <code>-V</code> and <code>--version</code> options should be
+used to output version information on stdout, which includes one line
+of the form '<code><em>Program</em> version
+<em>Major.Minor[.Patchlevel]</em> ... </code>'.  The program
+should exit successfully afterwards.
+</ul>
+
+When an unknown commandline argument is encountered, the program
+should display usage information on stderr and exit unsuccessfully.
 
 </body>
 </html>