I got tired of VISIBILITY_HIDDEN colliding with the gcc enum. Rename it
[oota-llvm.git] / docs / CodingStandards.html
index 6e1c240a000ec812fff3b465e2f8b7876baf1810..7815e19739f30854872e8501f3aacb22628937e1 100644 (file)
@@ -43,6 +43,8 @@
               Private</a></li>
           <li><a href="#hl_earlyexit">Use Early Exits and 'continue' to Simplify
               Code</a></li>
+          <li><a href="#hl_else_after_return">Don't use "else" after a
+              return</a></li>
           <li><a href="#hl_predicateloops">Turn Predicate Loops into Predicate
               Functions</a></li>
         </ol></li>
@@ -57,6 +59,7 @@
           <li><a href="#ll_iostream"><tt>#include &lt;iostream&gt;</tt> is
               <em>forbidden</em></a></li>
           <li><a href="#ll_avoidendl">Avoid <tt>std::endl</tt></a></li>
+          <li><a href="#ll_raw_ostream">Use <tt>raw_ostream</tt></a</li>
         </ol></li>
         
       <li><a href="#nano">Microscopic Details</a>
@@ -191,9 +194,9 @@ something sane goes a long ways towards avoiding writing documentation.</p>
 <b>Method information</b>
 
 <p>Methods defined in a class (as well as any global functions) should also be
-documented properly.  A quick note about what it does any a description of the
+documented properly.  A quick note about what it does and a description of the
 borderline behaviour is all that is necessary here (unless something
-particularly tricky or insideous is going on).  The hope is that people can
+particularly tricky or insidious is going on).  The hope is that people can
 figure out how to use your interfaces without reading the code itself... that is
 the goal metric.</p>
 
@@ -300,7 +303,7 @@ for debate.</p>
 <div class="doc_text">
 
 <p>In all cases, prefer spaces to tabs in source files.  People have different
-prefered indentation levels, and different styles of indentation that they
+preferred indentation levels, and different styles of indentation that they
 like... this is fine.  What isn't is that different editors/viewers expand tabs
 out to different tab stops.  This can cause your code to look completely
 unreadable, and it is not worth dealing with.</p>
@@ -416,7 +419,8 @@ different symbols based on whether <tt>class</tt> or <tt>struct</tt> was used to
 declare the symbol.  This can lead to problems at link time.</p> 
 
 <p>So, the rule for LLVM is to always use the <tt>class</tt> keyword, unless
-<b>all</b> members are public, in which case <tt>struct</tt> is allowed.</p>
+<b>all</b> members are public and the type is a C++ "POD" type, in which case 
+<tt>struct</tt> is allowed.</p>
 
 </div>
 
@@ -487,7 +491,7 @@ most cases, you simply don't need the definition of a class... and not
 <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
+accidentally 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
 above).  This way there won't be any hidden dependencies that you'll find out
 about later...</p>
@@ -623,6 +627,88 @@ be a big understandability win.</p>
 
 </div>
 
+<!-- _______________________________________________________________________ -->
+<div class="doc_subsubsection">
+  <a name="hl_else_after_return">Don't use "else" after a return</a>
+</div>
+
+<div class="doc_text">
+
+<p>For similar reasons above (reduction of indentation and easier reading),
+   please do not use "else" or "else if" after something that interrupts
+   control flow like return, break, continue, goto, etc.  For example, this is
+   "bad":</p>
+   
+<div class="doc_code">
+<pre>
+  case 'J': {
+    if (Signed) {
+      Type = Context.getsigjmp_bufType();
+      if (Type.isNull()) {
+        Error = ASTContext::GE_Missing_sigjmp_buf;
+        return QualType();
+      } else {
+        break;
+      }
+    } else {
+      Type = Context.getjmp_bufType();
+      if (Type.isNull()) {
+        Error = ASTContext::GE_Missing_jmp_buf;
+        return QualType();
+      } else {
+        break;
+      }
+    }
+  }
+  }
+</pre>
+</div>
+
+<p>It is better to write this something like:</p>
+
+<div class="doc_code">
+<pre>
+  case 'J':
+    if (Signed) {
+      Type = Context.getsigjmp_bufType();
+      if (Type.isNull()) {
+        Error = ASTContext::GE_Missing_sigjmp_buf;
+        return QualType();
+      }
+    } else {
+      Type = Context.getjmp_bufType();
+      if (Type.isNull()) {
+        Error = ASTContext::GE_Missing_jmp_buf;
+        return QualType();
+      }
+    }
+    break;
+</pre>
+</div>
+
+<p>Or better yet (in this case), as:</p>
+
+<div class="doc_code">
+<pre>
+  case 'J':
+    if (Signed)
+      Type = Context.getsigjmp_bufType();
+    else
+      Type = Context.getjmp_bufType();
+    
+    if (Type.isNull()) {
+      Error = Signed ? ASTContext::GE_Missing_sigjmp_buf :
+                       ASTContext::GE_Missing_jmp_buf;
+      return QualType();
+    }
+    break;
+</pre>
+</div>
+
+<p>The idea is to reduce indentation and the amount of code you have to keep
+   track of when reading the code.</p>
+              
+</div>
 
 <!-- _______________________________________________________________________ -->
 <div class="doc_subsubsection">
@@ -631,7 +717,7 @@ be a big understandability win.</p>
 
 <div class="doc_text">
 
-<p>It is very common to write inline functions that just compute a boolean
+<p>It is very common to write small loops that just compute a boolean
    value.  There are a number of ways that people commonly write these, but an
    example of this sort of thing is:</p>
    
@@ -653,8 +739,8 @@ be a big understandability win.</p>
 <p>This sort of code is awkward to write, and is almost always a bad sign.
 Instead of this sort of loop, we strongly prefer to use a predicate function
 (which may be <a href="#micro_anonns">static</a>) that uses
-<a href="#hl_earlyexit">early exits</a> to compute the predicate.  Code like
-this would be preferred:
+<a href="#hl_earlyexit">early exits</a> to compute the predicate.  We prefer
+the code to be structured like this:
 </p>
 
 
@@ -682,7 +768,7 @@ More importantly, it <em>forces you to pick a name</em> for the function, and
 forces you to write a comment for it.  In this silly example, this doesn't add
 much value.  However, if the condition is complex, this can make it a lot easier
 for the reader to understand the code that queries for this predicate.  Instead
-of being faced with the in-line details of we check to see if the BarList
+of being faced with the in-line details of how we check to see if the BarList
 contains a foo, we can trust the function name and continue reading with better
 locality.</p>
 
@@ -704,7 +790,7 @@ locality.</p>
 <div class="doc_text">
 
 <p>Use the "<tt>assert</tt>" function to its fullest.  Check all of your
-preconditions and assumptions, you never know when a bug (not neccesarily even
+preconditions and assumptions, you never know when a bug (not necessarily even
 yours) might be caught early by an assertion, which reduces debugging time
 dramatically.  The "<tt>&lt;cassert&gt;</tt>" header file is probably already
 included by the header files you are using, so it doesn't cost anything to use
@@ -904,12 +990,14 @@ library. There are two problems with this:</p>
 </ol>
 
 <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>
+example) is not problematic in this regard (just <tt>&lt;iostream&gt;</tt>).
+However, raw_ostream provides various APIs that are better performing for almost
+every use than std::ostream style APIs, so you should just use it for new
+code.</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>
+<p><b>New code should always
+use <a href="#ll_raw_ostream"><tt>raw_ostream</tt></a> for writing, or
+the <tt>llvm::MemoryBuffer</tt> API for reading files.</b></p>
 
 </div>
 
@@ -938,6 +1026,26 @@ it's better to use a literal <tt>'\n'</tt>.</p>
 </div>
 
 
+<!-- _______________________________________________________________________ -->
+<div class="doc_subsubsection">
+  <a name="ll_raw_ostream">Use <tt>raw_ostream</tt></a>
+</div>
+
+<div class="doc_text">
+
+<p>LLVM includes a lightweight, simple, and efficient stream implementation
+in <tt>llvm/Support/raw_ostream.h</tt> which provides all of the common features
+of <tt>std::ostream</tt>.  All new code should use <tt>raw_ostream</tt> instead
+of <tt>ostream</tt>.</p>
+
+<p>Unlike <tt>std::ostream</tt>, <tt>raw_ostream</tt> is not a template and can
+be forward declared as <tt>class raw_ostream</tt>.  Public headers should
+generally not include the <tt>raw_ostream</tt> header, but use forward
+declarations and constant references to <tt>raw_ostream</tt> instances.</p>
+
+</div>
+
+
 <!-- ======================================================================= -->
 <div class="doc_subsection">
   <a name="nano">Microscopic Details</a>
@@ -1050,21 +1158,27 @@ example:
 
 <div class="doc_code">
 <pre>
-/// SomeCrazyThing - This namespace contains flags for ...
-namespace SomeCrazyThing {
-  enum foo {
-    /// X - This is the X flag, which is ...
-    X = 1, 
-    
-    /// Y - This is the Y flag, which is ...
-    Y = 2,
-    
-    /// Z - This is the Z flag, which is ...
-    Z = 4,
-    
-    /// ALL_FLAGS - This is the union of all flags.
-    ALL_FLAGS = 7
-  };
+namespace llvm {
+  namespace X86 {
+    /// RelocationType - An enum for the x86 relocation codes. Note that
+    /// the terminology here doesn't follow x86 convention - word means
+    /// 32-bit and dword means 64-bit.
+    enum RelocationType {
+      /// reloc_pcrel_word - PC relative relocation, add the relocated value to
+      /// the value already in memory, after we adjust it for where the PC is.
+      reloc_pcrel_word = 0,
+
+      /// reloc_picrel_word - PIC base relative relocation, add the relocated
+      /// value to the value already in memory, after we adjust it for where the
+      /// PIC base is.
+      reloc_picrel_word = 1,
+      
+      /// reloc_absolute_word, reloc_absolute_dword - Absolute relocation, just
+      /// add the relocated value to the value already in memory.
+      reloc_absolute_word = 2,
+      reloc_absolute_dword = 3
+    };
+  }
 }
 </pre>
 </div>
@@ -1114,7 +1228,8 @@ the contents of the namespace.</p>
 
 <div class="doc_text">
 
-<p>A common topic after talking about namespaces is anonymous namespaces.
+<p>After talking about namespaces in general, you may be wondering about
+anonymous namespaces in particular.
 Anonymous namespaces are a great language feature that tells the C++ compiler
 that the contents of the namespace are only visible within the current
 translation unit, allowing more aggressive optimization and eliminating the
@@ -1186,7 +1301,7 @@ bool StringSort::operator&lt;(const char *RHS) const {
 of a large C++ file, that you have no immediate way to tell if it is local to
 the file.  When it is marked static explicitly, this is immediately obvious.
 Also, there is no reason to enclose the definition of "operator&lt;" in the
-namespace since it was declared there.
+namespace just because it was declared there.
 </p>
 
 </div>