assert(0) -> LLVM_UNREACHABLE.
[oota-llvm.git] / docs / CodingStandards.html
index 40d08383a8381bd75cf2916d77e72a9a921af326..dbe95a8d58815e0c14b1ed1cbac9218719820d46 100644 (file)
@@ -3,12 +3,12 @@
 <html>
 <head>
   <link rel="stylesheet" href="llvm.css" type="text/css">
-  <title>A Few Coding Standards</title>
+  <title>LLVM Coding Standards</title>
 </head>
 <body>
 
 <div class="doc_title">
-  A Few Coding Standards
+  LLVM Coding Standards
 </div>
 
 <ol>
@@ -49,7 +49,9 @@
           <li><a href="#ll_assert">Assert Liberally</a></li>
           <li><a href="#ll_ns_std">Do not use 'using namespace std'</a></li>
           <li><a href="#ll_virtual_anch">Provide a virtual method anchor for
-              clases in headers</a></li>
+              classes in headers</a></li>
+          <li><a href="#ll_end">Don't evaluate end() every time through a
+              loop</a></li>
           <li><a href="#ll_preincrement">Prefer Preincrement</a></li>
           <li><a href="#ll_avoidendl">Avoid <tt>std::endl</tt></a></li>
         </ol></li>
@@ -122,21 +124,21 @@ documentation is very useful:</p>
 
 <b>File Headers</b>
 
-<p>Every source file should have a header on it that
-describes the basic purpose of the file.  If a file does not have a header, it
-should not be checked into CVS.  Most source trees will probably have a standard
+<p>Every source file should have a header on it that describes the basic 
+purpose of the file.  If a file does not have a header, it should not be 
+checked into Subversion.  Most source trees will probably have a standard
 file header format.  The standard format for the LLVM source tree looks like
 this:</p>
 
 <div class="doc_code">
 <pre>
 //===-- llvm/Instruction.h - Instruction class definition -------*- C++ -*-===//
-// 
+//
 //                     The LLVM Compiler Infrastructure
 //
-// This file was developed by the LLVM research group and is distributed under
-// the University of Illinois Open Source License. See LICENSE.TXT for details.
-// 
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
 //===----------------------------------------------------------------------===//
 //
 // This file contains the declaration of the Instruction class, which is the
@@ -154,9 +156,9 @@ on the first line, along with a very short description of the purpose of the
 file.  This is important when printing out code and flipping though lots of
 pages.</p>
 
-<p>The next section in the file is a concise note that defines the license that
-the file is released under.  This makes it perfectly clear what terms the source
-code can be distributed under.</p>
+<p>The next section in the file is a concise note that defines the license
+that the file is released under.  This makes it perfectly clear what terms the
+source code can be distributed under and should not be modified in any way.</p>
 
 <p>The main body of the description does not have to be very long in most cases.
 Here it's only two lines.  If an algorithm is being implemented or something
@@ -238,7 +240,7 @@ order:</p>
   <li>System <tt>#includes</tt></li>
 </ol>
 
-<p>... and each catagory should be sorted by name.</p>
+<p>... and each category should be sorted by name.</p>
 
 <p><a name="mmheader">The "Main Module Header"</a> file applies to .cpp file
 which implement an interface defined by a .h file.  This <tt>#include</tt>
@@ -262,6 +264,18 @@ implements are defined.</p>
 like to print out code and look at your code in an xterm without resizing
 it.</p>
 
+<p>The longer answer is that there must be some limit to the width of the code
+in order to reasonably allow developers to have multiple files side-by-side in
+windows on a modest display.  If you are going to pick a width limit, it is
+somewhat arbitrary but you might as well pick something standard.  Going with
+90 columns (for example) instead of 80 columns wouldn't add any significant 
+value and would be detrimental to printing out code.  Also many other projects
+have standardized on 80 columns, so some people have already configured their
+editors for it (vs something else, like 90 columns).</p>
+
+<p>This is one of many contentious issues in coding standards, but is not up
+for debate.</p>
+
 </div>
 
 <!-- _______________________________________________________________________ -->
@@ -367,9 +381,8 @@ code, isolate it behind a well defined (and well documented) interface.</p>
 
 <p>In practice, this means that you shouldn't assume much about the host
 compiler, including its support for "high tech" features like partial
-specialization of templates.  In fact, Visual C++ 6 could be an important target
-for our work in the future, and we don't want to have to rewrite all of our code
-to support it.</p>
+specialization of templates.  If these features are used, they should only be
+an implementation detail of a library which has a simple exposed API.</p>
 
 </div>
 
@@ -456,7 +469,8 @@ most cases, you simply don't need the definition of a class... and not
 <tt>#include</tt>'ing speeds up compilation.</p>
 
 <p>It is easy to try to go too overboard on this recommendation, however.  You
-<b>must</b> include all of the header files that you are using, either directly
+<b>must</b> include all of the header files that you are using -- you can 
+include them either directly
 or indirectly (through another header file).  To make sure that you don't
 accidently forget to include a header file in your module header, make sure to
 include your module header <b>first</b> in the implementation file (as mentioned
@@ -502,53 +516,20 @@ library. There are two problems with this:</p>
 
 <ol>
   <li>The time to run the static c'tors impacts startup time of
-      applications&mdash;a critical time for gui apps.</li>
+      applications&mdash;a critical time for GUI apps.</li>
   <li>The static c'tors cause the app to pull many extra pages of memory off the
-      disk: both the code for the static c'tors in each .o file and the small
-      amount of data that gets touched.  In addition, touched/dirty pages put
-      more pressure on the VM system on low-memory machines.</li>
+      disk: both the code for the static c'tors in each <tt>.o</tt> file and the
+      small amount of data that gets touched. In addition, touched/dirty pages
+      put more pressure on the VM system on low-memory machines.</li>
 </ol>
 
-<table>
-  <tbody>
-    <tr>
-      <th>Old Way</th>
-      <th>New Way</th>
-    </tr>
-    <tr>
-      <td align="left"><pre>#include &lt;iostream&gt;</pre></td>
-      <td align="left"><pre>#include "llvm/Support/Streams.h"</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>DEBUG(std::cerr &lt;&lt; ...);</pre></td>
-      <td align="left"><pre>DOUT &lt;&lt; ...;</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::cerr &lt;&lt; "Hello world\n";</pre></td>
-      <td align="left"><pre>llvm::cerr &lt;&lt; "Hello world\n";</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::cout &lt;&lt; "Hello world\n";</pre></td>
-      <td align="left"><pre>llvm::cout &lt;&lt; "Hello world\n";</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::cin &gt;&gt; Var;</pre></td>
-      <td align="left"><pre>llvm::cin &gt;&gt; Var;</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::ostream</pre></td>
-      <td align="left"><pre>llvm::OStream</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::istream</pre></td>
-      <td align="left"><pre>llvm::IStream</pre></td>
-    </tr>
-    <tr>
-      <td align="left"><pre>std::stringstream</pre></td>
-      <td align="left"><pre>llvm::StringStream</pre></td>
-    </tr>
-  </tbody>
-</table>
+<p>Note that using the other stream headers (<tt>&lt;sstream&gt;</tt> for
+example) is allowed normally, it is just <tt>&lt;iostream&gt;</tt> that is
+causing problems.</p>
+
+<p>The preferred replacement for stream functionality is the
+<tt>llvm::raw_ostream</tt> class (for writing to output streams of various
+sorts) and the <tt>llvm::MemoryBuffer</tt> API (for reading in files).</p>
 
 </div>
 
@@ -605,46 +586,70 @@ assert(isa&lt;PHINode&gt;(Succ-&gt;front()) &amp;&amp; "Only works on PHId BBs!"
 
 <p>You get the idea...</p>
 
+<p>Please be aware when adding assert statements that not all compilers are aware of
+the semantics of the assert.  In some places, asserts are used to indicate a piece of
+code that should not be reached.  These are typically of the form:</p>
+
+<div class="doc_code">
+<pre>
+assert(0 &amp;&amp; "Some helpful error message");
+</pre>
+</div>
+
+<p>When used in a function that returns a value, they should be followed with a return
+statement and a comment indicating that this line is never reached.  This will prevent
+a compiler which is unable to deduce that the assert statement never returns from
+generating a warning.</p>
+
+<div class="doc_code">
+<pre>
+assert(0 &amp;&amp; "Some helpful error message");
+// Not reached
+return 0;
+</pre>
+</div>
+
 </div>
 
 <!-- _______________________________________________________________________ -->
 <div class="doc_subsubsection">
-  <a name="ll_ns_std">Do not use 'using namespace std'</a>
+  <a name="ll_ns_std">Do not use '<tt>using namespace std</tt>'</a>
 </div>
 
 <div class="doc_text">
 <p>In LLVM, we prefer to explicitly prefix all identifiers from the standard
-namespace with an "std::" prefix, rather than rely on "using namespace std;".
-</p>
-
-<p> In header files, adding a 'using namespace XXX' directive pollutes the 
-namespace of any source file that includes the header.  This is clearly a bad
-thing.</p>
-
-<p>In implementation files (e.g. .cpp files) the rule is more of a stylistic
-rule, but is still important.  Basically, using explicit namespace prefixes 
-makes
-the code <b>more clear</b> - because it is immediately obvious what facilities
-are being used and where they are coming from - and <b>more portable</b> -
-because namespace clashes cannot occur between LLVM code and other namespaces.
-The portability rule is important because different standard library 
-implementations expose different symbols (potentially ones they shouldn't) and 
-future revisions to the C++ standard will add more symbols to the std 
-namespace.  As such, we never 'using namespace std;' in LLVM.</p>
-
-<p>The exception to the general rule (i.e. it's not an exception for the std 
-namespace) is for implementation files.  For example, all of the code in the
-LLVM project implements code that lives in the 'llvm' namespace.  As such, it
-is ok, and actually more clear, for the .cpp files to have a 'using namespace 
-llvm' directive at their top, after the #includes.  The general form of this
-rule is that any .cpp file that implements code in any namespace may use that
-namespace (and its parents), but should not use any others.</p>
+namespace with an "<tt>std::</tt>" prefix, rather than rely on
+"<tt>using namespace std;</tt>".</p>
+
+<p> In header files, adding a '<tt>using namespace XXX</tt>' directive pollutes
+the namespace of any source file that <tt>#include</tt>s the header.  This is
+clearly a bad thing.</p>
+
+<p>In implementation files (e.g. .cpp files), the rule is more of a stylistic
+rule, but is still important.  Basically, using explicit namespace prefixes
+makes the code <b>clearer</b>, because it is immediately obvious what facilities
+are being used and where they are coming from, and <b>more portable</b>, because
+namespace clashes cannot occur between LLVM code and other namespaces.  The
+portability rule is important because different standard library implementations
+expose different symbols (potentially ones they shouldn't), and future revisions
+to the C++ standard will add more symbols to the <tt>std</tt> namespace.  As
+such, we never use '<tt>using namespace std;</tt>' in LLVM.</p>
+
+<p>The exception to the general rule (i.e. it's not an exception for
+the <tt>std</tt> namespace) is for implementation files.  For example, all of
+the code in the LLVM project implements code that lives in the 'llvm' namespace.
+As such, it is ok, and actually clearer, for the .cpp files to have a '<tt>using
+namespace llvm</tt>' directive at their top, after the <tt>#include</tt>s.  The
+general form of this rule is that any .cpp file that implements code in any
+namespace may use that namespace (and its parents'), but should not use any
+others.</p>
 
 </div>
 
 <!-- _______________________________________________________________________ -->
 <div class="doc_subsubsection">
-  <a name="ll_virtual_anch">Provide a virtual method anchor for clases in headers</a>
+  <a name="ll_virtual_anch">Provide a virtual method anchor for classes
+  in headers</a>
 </div>
 
 <div class="doc_text">
@@ -652,9 +657,70 @@ namespace (and its parents), but should not use any others.</p>
 <p>If a class is defined in a header file and has a v-table (either it has 
 virtual methods or it derives from classes with virtual methods), it must 
 always have at least one out-of-line virtual method in the class.  Without 
-this, the compiler will copy the vtable and RTTI into every .o file that
-#includes the header, bloating .o file sizes and increasing link times.
-</p>
+this, the compiler will copy the vtable and RTTI into every <tt>.o</tt> file
+that <tt>#include</tt>s the header, bloating <tt>.o</tt> file sizes and
+increasing link times.</p>
+
+</div>
+
+<!-- _______________________________________________________________________ -->
+<div class="doc_subsubsection">
+  <a name="ll_end">Don't evaluate end() every time through a loop</a>
+</div>
+
+<div class="doc_text">
+
+<p>Because C++ doesn't have a standard "foreach" loop (though it can be emulated
+with macros and may be coming in C++'0x) we end up writing a lot of loops that
+manually iterate from begin to end on a variety of containers or through other
+data structures.  One common mistake is to write a loop in this style:</p>
+
+<div class="doc_code">
+<pre>
+  BasicBlock *BB = ...
+  for (BasicBlock::iterator I = BB->begin(); I != <b>BB->end()</b>; ++I)
+     ... use I ...
+</pre>
+</div>
+
+<p>The problem with this construct is that it evaluates "<tt>BB->end()</tt>"
+every time through the loop.  Instead of writing the loop like this, we strongly
+prefer loops to be written so that they evaluate it once before the loop starts.
+A convenient way to do this is like so:</p>
+
+<div class="doc_code">
+<pre>
+  BasicBlock *BB = ...
+  for (BasicBlock::iterator I = BB->begin(), E = <b>BB->end()</b>; I != E; ++I)
+     ... use I ...
+</pre>
+</div>
+
+<p>The observant may quickly point out that these two loops may have different
+semantics: if the container (a basic block in this case) is being mutated, then
+"<tt>BB->end()</tt>" may change its value every time through the loop and the
+second loop may not in fact be correct.  If you actually do depend on this
+behavior, please write the loop in the first form and add a comment indicating
+that you did it intentionally.</p>
+
+<p>Why do we prefer the second form (when correct)?  Writing the loop in the
+first form has two problems: First it may be less efficient than evaluating it
+at the start of the loop.  In this case, the cost is probably minor: a few extra
+loads every time through the loop.  However, if the base expression is more
+complex, then the cost can rise quickly.  I've seen loops where the end
+expression was actually something like: "<tt>SomeMap[x]->end()</tt>" and map
+lookups really aren't cheap.  By writing it in the second form consistently, you
+eliminate the issue entirely and don't even have to think about it.</p>
+
+<p>The second (even bigger) issue is that writing the loop in the first form
+hints to the reader that the loop is mutating the container (a fact that a
+comment would handily confirm!).  If you write the loop in the second form, it
+is immediately obvious without even looking at the body of the loop that the
+container isn't being modified, which makes it easier to read the code and
+understand what it does.</p>
+
+<p>While the second form of the loop is a few extra keystrokes, we do strongly
+prefer it.</p>
 
 </div>
 
@@ -716,15 +782,12 @@ sources.  Two particularly important books for our work are:</p>
 
 <ol>
 
-<li><a href="http://www.aw-bc.com/catalog/academic/product/0,1144,0201310155,00.html">Effective
-C++</a> by Scott Meyers.  There is an online version of the book (only some
-chapters though) <a
-href="http://www.awlonline.com/cseng/meyerscddemo/">available as well</a>.  Also
+<li><a href="http://www.amazon.com/Effective-Specific-Addison-Wesley-Professional-Computing/dp/0321334876">Effective
+C++</a> by Scott Meyers.  Also 
 interesting and useful are "More Effective C++" and "Effective STL" by the same
 author.</li>
 
-<li><a href="http://cseng.aw.com/book/0,3828,0201633620,00.html">Large-Scale C++
-Software Design</a> by John Lakos</li>
+<li>Large-Scale C++ Software Design by John Lakos</li>
 
 </ol>
 
@@ -738,9 +801,9 @@ something.</p>
 <hr>
 <address>
   <a href="http://jigsaw.w3.org/css-validator/check/referer"><img
-  src="http://jigsaw.w3.org/css-validator/images/vcss" alt="Valid CSS!"></a>
+  src="http://jigsaw.w3.org/css-validator/images/vcss-blue" alt="Valid CSS"></a>
   <a href="http://validator.w3.org/check/referer"><img
-  src="http://www.w3.org/Icons/valid-html401" alt="Valid HTML 4.01!"></a>
+  src="http://www.w3.org/Icons/valid-html401-blue" alt="Valid HTML 4.01"></a>
 
   <a href="mailto:sabre@nondot.org">Chris Lattner</a><br>
   <a href="http://llvm.org">LLVM Compiler Infrastructure</a><br>