LLVM Coding Standards

+ +

This document attempts to describe a few coding standards that are being used +in the LLVM source tree. Although no coding standards should be regarded as +absolute requirements to be followed in all instances, coding standards can be +useful.

+ +

This document intentionally does not prescribe fixed standards for religious +issues such as brace placement and space usage. For issues like this, follow +the golden rule:

+ +

+ +
If you are adding a significant body of source to a +project, feel free to use whatever style you are most comfortable with. If you +are extending, enhancing, or bug fixing already implemented code, use the style +that is already being used so that the source is uniform and easy to +follow.
+ +

+ +

The ultimate goal of these guidelines is the increase readability and +maintainability of our common source base. If you have suggestions for topics to +be included, please mail them to Chris.

+ +

Comments are one critical part of readability and maintainability. Everyone +knows they should comment, so should you. When writing comments, write them as +English prose, which means they should use proper capitalization, punctuation, +etc. Although we all should probably +comment our code more than we do, there are a few very critical places that +documentation is very useful:

+ +File Headers + +

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:

+ +

+//===-- llvm/Instruction.h - Instruction class definition -------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// 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
+// base class for all of the VM instructions.
+//
+//===----------------------------------------------------------------------===//
+

+ +

A few things to note about this particular format: The "-*- C++ +-*-" string on the first line is there to tell Emacs that the source file +is a C++ file, not a C file (Emacs assumes .h files are C files by default). +Note that this tag is not necessary in .cpp files. The name of the file is also +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.

+ +

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.

+ +

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 +tricky is going on, a reference to the paper where it is published should be +included, as well as any notes or "gotchas" in the code to watch out for.

+ +Class overviews + +

Classes are one fundamental part of a good object oriented design. As such, +a class definition should have a comment block that explains what the class is +used for... if it's not obvious. If it's so completely obvious your grandma +could figure it out, it's probably safe to leave it out. Naming classes +something sane goes a long ways towards avoiding writing documentation.

+ + +Method information + +

Methods defined in a class (as well as any global functions) should also be +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 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.

+ +

Good things to talk about here are what happens when something unexpected +happens: does the method return null? Abort? Format your hard disk?

+ +

In general, prefer C++ style (//) comments. They take less space, +require less typing, don't have nesting problems, etc. There are a few cases +when it is useful to use C style (/* */) comments however:

+ +

When writing a C code: Obviously if you are writing C code, use C style + comments.
When writing a header file that may be #included by a C source + file.
When writing a source file that is used by a tool that only accepts C + style comments.

+ +

To comment out a large block of code, use #if 0 and #endif. +These nest properly and are better behaved in general than C style comments.

+ +

Immediately after the header file comment (and +include guards if working on a header file), the minimal list of #includes required by the +file should be listed. We prefer these #includes to be listed in this +order:

+ +

Main Module header
Local/Private Headers
llvm/*
llvm/Analysis/*
llvm/Assembly/*
llvm/Bytecode/*
llvm/CodeGen/*
...
Support/*
Config/*
System #includes

+ +

... and each category should be sorted by name.

+ +

The "Main Module Header" file applies to .cpp file +which implement an interface defined by a .h file. This #include +should always be included first regardless of where it lives on the file +system. By including a header file first in the .cpp files that implement the +interfaces, we ensure that the header does not have any hidden dependencies +which are not explicitly #included in the header, but should be. It is also a +form of documentation in the .cpp file to indicate where the interfaces it +implements are defined.

+ +

Write your code to fit within 80 columns of text. This helps those of us who +like to print out code and look at your code in an xterm without resizing +it.

+ +

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).

+ +

This is one of many contentious issues in coding standards, but is not up +for debate.

+ +

In all cases, prefer spaces to tabs in source files. People have different +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.

+ +

As always, follow the Golden Rule above: follow the +style of existing code if your are modifying and extending it. If you like four +spaces of indentation, DO NOT do that in the middle of a chunk of code +with two spaces of indentation. Also, do not reindent a whole source file: it +makes for incredible diffs that are absolutely worthless.

+ +

Okay, your first year of programming you were told that indentation is +important. If you didn't believe and internalize this then, now is the time. +Just do it.

+ +

If your code has compiler warnings in it, something is wrong: you aren't +casting values correctly, your have "questionable" constructs in your code, or +you are doing something legitimately wrong. Compiler warnings can cover up +legitimate errors in output and make dealing with a translation unit +difficult.

+ +

It is not possible to prevent all warnings from all compilers, nor is it +desirable. Instead, pick a standard compiler (like gcc) that provides +a good thorough set of warnings, and stick to them. At least in the case of +gcc, it is possible to work around any spurious errors by changing the +syntax of the code slightly. For example, an warning that annoys me occurs when +I write code like this:

+ +

+if (V = getValue()) {
+  ...
+}
+

+ +

gcc will warn me that I probably want to use the == +operator, and that I probably mistyped it. In most cases, I haven't, and I +really don't want the spurious errors. To fix this particular problem, I +rewrite the code like this:

+ +

+if ((V = getValue())) {
+  ...
+}
+

+ +

...which shuts gcc up. Any gcc warning that annoys you can +be fixed by massaging the code appropriately.

+ +

These are the gcc warnings that I prefer to enable: -Wall +-Winline -W -Wwrite-strings -Wno-unused

+ +

In almost all cases, it is possible and within reason to write completely +portable code. If there are cases where it isn't possible to write portable +code, isolate it behind a well defined (and well documented) interface.

+ +

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. If these features are used, they should only be +an implementation detail of a library which has a simple exposed API.

+ +

In C++, the class and struct keywords can be used almost +interchangeably. The only difference is when they are used to declare a class: +class makes all members private by default while struct makes +all members public by default.

+ +

Unfortunately, not all compilers follow the rules and some will generate +different symbols based on whether class or struct was used to +declare the symbol. This can lead to problems at link time.

+ +

So, the rule for LLVM is to always use the class keyword, unless +all members are public and the type is a C++ "POD" type, in which case +struct is allowed.

+ +

C++ doesn't do too well in the modularity department. There is no real +encapsulation or data hiding (unless you use expensive protocol classes), but it +is what we have to work with. When you write a public header file (in the LLVM +source tree, they live in the top level "include" directory), you are defining a +module of functionality.

+ +

Ideally, modules should be completely independent of each other, and their +header files should only include the absolute minimum number of headers +possible. A module is not just a class, a function, or a namespace: it's a collection +of these that defines an interface. This interface may be several +functions, classes or data structures, but the important issue is how they work +together.

+ +

In general, a module should be implemented with one or more .cpp +files. Each of these .cpp files should include the header that defines +their interface first. This ensure that all of the dependences of the module +header have been properly added to the module header itself, and are not +implicit. System headers should be included after user headers for a +translation unit.

+ +

#include hurts compile time performance. Don't do it unless you +have to, especially in header files.

+ +

But wait, sometimes you need to have the definition of a class to use it, or +to inherit from it. In these cases go ahead and #include that header +file. Be aware however that there are many cases where you don't need to have +the full definition of a class. If you are using a pointer or reference to a +class, you don't need the header file. If you are simply returning a class +instance from a prototyped function or method, you don't need it. In fact, for +most cases, you simply don't need the definition of a class... and not +#include'ing speeds up compilation.

+ +

It is easy to try to go too overboard on this recommendation, however. You +must 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 +accidentally forget to include a header file in your module header, make sure to +include your module header first in the implementation file (as mentioned +above). This way there won't be any hidden dependencies that you'll find out +about later...

+ +

Many modules have a complex implementation that causes them to use more than +one implementation (.cpp) file. It is often tempting to put the +internal communication interface (helper classes, extra functions, etc) in the +public module header file. Don't do this.

+ +

If you really need to do something like this, put a private header file in +the same directory as the source files, and include it locally. This ensures +that your private interface remains private and undisturbed by outsiders.

+ +

Note however, that it's okay to put extra implementation methods a public +class itself... just make them private (or protected), and all is well.

+ +

When reading code, keep in mind how much state and how many previous +decisions have to be remembered by the reader to understand a block of code. +Aim to reduce indentation where possible when it doesn't make it more difficult +to understand the code. One great way to do this is by making use of early +exits and the 'continue' keyword in long loops. As an example of using an early +exit from a function, consider this "bad" code:

+ +

+Value *DoSomething(Instruction *I) {
+  if (!isa<TerminatorInst>(I) &&
+      I->hasOneUse() && SomeOtherThing(I)) {
+    ... some long code ....
+  }
+  
+  return 0;
+}
+

+ +

This code has several problems if the body of the 'if' is large. When you're +looking at the top of the function, it isn't immediately clear that this +only does interesting things with non-terminator instructions, and only +applies to things with the other predicates. Second, it is relatively difficult +to describe (in comments) why these predicates are important because the if +statement makes it difficult to lay out the comments. Third, when you're deep +within the body of the code, it is indented an extra level. Finally, when +reading the top of the function, it isn't clear what the result is if the +predicate isn't true, you have to read to the end of the function to know that +it returns null.

+ +

It is much preferred to format the code like this:

+ +

+Value *DoSomething(Instruction *I) {
+  // Terminators never need 'something' done to them because, ... 
+  if (isa<TerminatorInst>(I))
+    return 0;
+
+  // We conservatively avoid transforming instructions with multiple uses
+  // because goats like cheese.
+  if (!I->hasOneUse())
+    return 0;
+
+  // This is really just here for example.
+  if (!SomeOtherThing(I))
+    return 0;
+    
+  ... some long code ....
+}
+

+ +

This fixes these problems. A similar problem frequently happens in for +loops. A silly example is something like this:

+ +

+  for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) {
+    if (BinaryOperator *BO = dyn_cast<BinaryOperator>(II)) {
+      Value *LHS = BO->getOperand(0);
+      Value *RHS = BO->getOperand(1);
+      if (LHS != RHS) {
+        ...
+      }
+    }
+  }
+

+ +

When you have very very small loops, this sort of structure is fine, but if +it exceeds more than 10-15 lines, it becomes difficult for people to read and +understand at a glance. +The problem with this sort of code is that it gets very nested very quickly, +meaning that the reader of the code has to keep a lot of context in their brain +to remember what is going immediately on in the loop, because they don't know +if/when the if conditions will have elses etc. It is strongly preferred to +structure the loop like this:

+ +

+  for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) {
+    BinaryOperator *BO = dyn_cast<BinaryOperator>(II);
+    if (!BO) continue;
+    
+    Value *LHS = BO->getOperand(0);
+    Value *RHS = BO->getOperand(1);
+    if (LHS == RHS) continue;
+  }
+

+ +

This has all the benefits of using early exits from functions: it reduces +nesting of the loop, it makes it easier to describe why the conditions are true, +and it makes it obvious to the reader that there is no "else" coming up that +they have to push context into their brain for. If a loop is large, this can +be a big understandability win.

+ +

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":

+ +

+  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;
+      }
+    }
+  }
+  }
+

+ +

It is better to write this something like:

+ +

+  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;
+

+ +

Or better yet (in this case), as:

+ +

+  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;
+

+ +

The idea is to reduce indentation and the amount of code you have to keep + track of when reading the code.

+ +

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:

+ +

+  bool FoundFoo = false;
+  for (unsigned i = 0, e = BarList.size(); i != e; ++i)
+    if (BarList[i]->isFoo()) {
+      FoundFoo = true;
+      break;
+    }
+    
+  if (FoundFoo) {
+    ...
+  }
+

+ +

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 static) that uses +early exits to compute the predicate. We prefer +the code to be structured like this: +

+ + +

+/// ListContainsFoo - Return true if the specified list has an element that is
+/// a foo.
+static bool ListContainsFoo(const std::vector<Bar*> &List) {
+  for (unsigned i = 0, e = List.size(); i != e; ++i)
+    if (List[i]->isFoo())
+      return true;
+  return false;
+}
+...
+
+  if (ListContainsFoo(BarList)) {
+    ...
+  }
+

+ +

There are many reasons for doing this: it reduces indentation and factors out +code which can often be shared by other code that checks for the same predicate. +More importantly, it forces you to pick a name 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 how we check to see if the BarList +contains a foo, we can trust the function name and continue reading with better +locality.

+ +

Use the "assert" function to its fullest. Check all of your +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 "<cassert>" header file is probably already +included by the header files you are using, so it doesn't cost anything to use +it.

+ +

To further assist with debugging, make sure to put some kind of error message +in the assertion statement (which is printed if the assertion is tripped). This +helps the poor debugging make sense of why an assertion is being made and +enforced, and hopefully what to do about it. Here is one complete example:

+ +

+inline Value *getOperand(unsigned i) { 
+  assert(i < Operands.size() && "getOperand() out of range!");
+  return Operands[i]; 
+}
+

+ +

Here are some examples:

+ +

+assert(Ty->isPointerType() && "Can't allocate a non pointer type!");
+
+assert((Opcode == Shl || Opcode == Shr) && "ShiftInst Opcode invalid!");
+
+assert(idx < getNumSuccessors() && "Successor # out of range!");
+
+assert(V1.getType() == V2.getType() && "Constant types must be identical!");
+
+assert(isa<PHINode>(Succ->front()) && "Only works on PHId BBs!");
+

+ +

You get the idea...

+ +

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:

+ +

+assert(0 && "Some helpful error message");
+

+ +

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.

+ +

+assert(0 && "Some helpful error message");
+// Not reached
+return 0;
+

+ +

In LLVM, we prefer to explicitly prefix all identifiers from the standard +namespace with an "std::" prefix, rather than rely on +"using namespace std;".

+ +

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.

+ +

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 clearer, because it is immediately obvious what facilities +are being used and where they are coming from, and more portable, 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 use 'using namespace std;' in LLVM.

+ +

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 clearer, 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.

+ +

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.

+ +

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:

+ +

+  BasicBlock *BB = ...
+  for (BasicBlock::iterator I = BB->begin(); I != BB->end(); ++I)
+     ... use I ...
+

+ +

The problem with this construct is that it evaluates "BB->end()" +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:

+ +

+  BasicBlock *BB = ...
+  for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I)
+     ... use I ...
+

+ +

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 +"BB->end()" 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.

+ +

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: "SomeMap[x]->end()" 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.

+ +

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.

+ +

While the second form of the loop is a few extra keystrokes, we do strongly +prefer it.

+ +

The use of #include <iostream> in library files is +hereby forbidden. The primary reason for doing this is to +support clients using LLVM libraries as part of larger systems. In particular, +we statically link LLVM into some dynamic libraries. Even if LLVM isn't used, +the static c'tors are run whenever an application start up that uses the dynamic +library. There are two problems with this:

+ +

The time to run the static c'tors impacts startup time of + applications—a critical time for GUI apps.
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.

+ +

Note that using the other stream headers (<sstream> for +example) is not problematic in this regard (just <iostream>). +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.

+ +

New code should always +use raw_ostream for writing, or +the llvm::MemoryBuffer API for reading files.

+ +

The std::endl modifier, when used with iostreams outputs a newline +to the output stream specified. In addition to doing this, however, it also +flushes the output stream. In other words, these are equivalent:

+ +

+std::cout << std::endl;
+std::cout << '\n' << std::flush;
+

+ +

Most of the time, you probably have no reason to flush the output stream, so +it's better to use a literal '\n'.

+ +

LLVM includes a lightweight, simple, and efficient stream implementation +in llvm/Support/raw_ostream.h which provides all of the common features +of std::ostream. All new code should use raw_ostream instead +of ostream.

+ +

Unlike std::ostream, raw_ostream is not a template and can +be forward declared as class raw_ostream. Public headers should +generally not include the raw_ostream header, but use forward +declarations and constant references to raw_ostream instances.

+ +

This section describes preferred low-level formatting guidelines along with +reasoning on why we prefer them.

+ +

We prefer to put a space before a parentheses only in control flow +statements, but not in normal function call expressions and function-like +macros. For example, this is good:

+ +

+  if (x) ...
+  for (i = 0; i != 100; ++i) ...
+  while (llvm_rocks) ...
+
+  somefunc(42);
+  assert(3 != 4 && "laws of math are failing me");
+  
+  a = foo(42, 92) + bar(x);
+

+ +

... and this is bad:

+ +

+  if(x) ...
+  for(i = 0; i != 100; ++i) ...
+  while(llvm_rocks) ...
+
+  somefunc (42);
+  assert (3 != 4 && "laws of math are failing me");
+  
+  a = foo (42, 92) + bar (x);
+

+ +

The reason for doing this is not completely arbitrary. This style makes + control flow operators stand out more, and makes expressions flow better. The + function call operator binds very tightly as a postfix operator. Putting + a space after a function name (as in the last example) makes it appear that + the code might bind the arguments of the left-hand-side of a binary operator + with the argument list of a function and the name of the right side. More + specifically, it is easy to misread the "a" example as:

+ +

+  a = foo ((42, 92) + bar) (x);
+

+ +

... when skimming through the code. By avoiding a space in a function, we +avoid this misinterpretation.

+ +

Hard fast rule: Preincrement (++X) may be no slower than +postincrement (X++) and could very well be a lot faster than it. Use +preincrementation whenever possible.

+ +

The semantics of postincrement include making a copy of the value being +incremented, returning it, and then preincrementing the "work value". For +primitive types, this isn't a big deal... but for iterators, it can be a huge +issue (for example, some iterators contains stack and set objects in them... +copying an iterator could invoke the copy ctor's of these as well). In general, +get in the habit of always using preincrement, and you won't have a problem.

+ +

+In general, we strive to reduce indentation where ever possible. This is useful +because we want code to fit into 80 columns without +wrapping horribly, but also because it makes it easier to understand the code. +Namespaces are a funny thing: they are often large, and we often desire to put +lots of stuff into them (so they can be large). Other times they are tiny, +because they just hold an enum or something similar. In order to balance this, +we use different approaches for small versus large namespaces. +

+ +

+If a namespace definition is small and easily fits on a screen (say, +less than 35 lines of code), then you should indent its body. Here's an +example: +

+ +

+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
+    };
+  }
+}
+

+ +

Since the body is small, indenting adds value because it makes it very clear +where the namespace starts and ends, and it is easy to take the whole thing in +in one "gulp" when reading the code. If the blob of code in the namespace is +larger (as it typically is in a header in the llvm or clang namespaces), do not +indent the code, and add a comment indicating what namespace is being closed. +For example:

+ +

+namespace llvm {
+namespace knowledge {
+
+/// Grokable - This class represents things that Smith can have an intimate
+/// understanding of and contains the data associated with it.
+class Grokable {
+...
+public:
+  explicit Grokable() { ... }
+  virtual ~Grokable() = 0;
+  
+  ...
+
+};
+
+} // end namespace knowledge
+} // end namespace llvm
+

+ +

Because the class is large, we don't expect that the reader can easily +understand the entire concept in a glance, and the end of the file (where the +namespaces end) may be a long ways away from the place they open. As such, +indenting the contents of the namespace doesn't add any value, and detracts from +the readability of the class. In these cases it is best to not indent +the contents of the namespace.

+ +

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 +possibility of symbol name collisions. Anonymous namespaces are to C++ as +"static" is to C functions and global variables. While "static" is available +in C++, anonymous namespaces are more general: they can make entire classes +private to a file.

+ +

The problem with anonymous namespaces is that they naturally want to +encourage indentation of their body, and they reduce locality of reference: if +you see a random function definition in a C++ file, it is easy to see if it is +marked static, but seeing if it is in an anonymous namespace requires scanning +a big chunk of the file.

+ +

Because of this, we have a simple guideline: make anonymous namespaces as +small as possible, and only use them for class declarations. For example, this +is good:

+ +

+namespace {
+  class StringSort {
+  ...
+  public:
+    StringSort(...)
+    bool operator<(const char *RHS) const;
+  };
+} // end anonymous namespace
+
+static void Helper() { 
+  ... 
+}
+
+bool StringSort::operator<(const char *RHS) const {
+  ...
+}
+
+

+ +

This is bad:

+ + +

+namespace {
+class StringSort {
+...
+public:
+  StringSort(...)
+  bool operator<(const char *RHS) const;
+};
+
+void Helper() { 
+  ... 
+}
+
+bool StringSort::operator<(const char *RHS) const {
+  ...
+}
+
+} // end anonymous namespace
+
+

+ + +

This is bad specifically because if you're looking at "Helper" in the middle +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<" in the +namespace just because it was declared there. +

+ +