[llvm] r199637 - Revert my commit in r199620 that added sections about namespaces to the

Chandler Carruth chandlerc at gmail.com
Mon Jan 20 02:15:32 PST 2014


Author: chandlerc
Date: Mon Jan 20 04:15:32 2014
New Revision: 199637

URL: http://llvm.org/viewvc/llvm-project?rev=199637&view=rev
Log:
Revert my commit in r199620 that added sections about namespaces to the
coding standards, and instead fix the existing section.

Thanks to Daniel Jasper for pointing out we already had a section
devoted to this topic. Instead of adding sections, just hack on this
section some. Also fix the example in the anonymous namespace section
below it to agree with the new advice.

As a re-cap, this switches the LLVM preferred style to never indent
namespaces. Having two approaches just led to endless (and utterly
pointless) debates about what was "small enough". This wasn't helping
anyone. The no-indent rule is easy to understand and doesn't really make
anything harder to read. Moreover, with tools like clang-format it is
considerably nicer to have simple consistent rules.

Modified:
    llvm/trunk/docs/CodingStandards.rst

Modified: llvm/trunk/docs/CodingStandards.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CodingStandards.rst?rev=199637&r1=199636&r2=199637&view=diff
==============================================================================
--- llvm/trunk/docs/CodingStandards.rst (original)
+++ llvm/trunk/docs/CodingStandards.rst Mon Jan 20 04:15:32 2014
@@ -106,24 +106,6 @@ an algorithm is being implemented or som
 to the paper where it is published should be included, as well as any notes or
 *gotchas* in the code to watch out for.
 
-Namespace Markers
-"""""""""""""""""
-
-We don't indent namespaces (see below) and so feel free to add markers to the
-end of a namespace where it helps readabilitily:
-
-.. code-block:: c++
-
-  namespace foo {
-
-  // Lots of code here...
-
-  } // End foo namespace
-
-This isn't required, and in many cases (such as the namespace used for an
-entire file like the 'llvm' namespace in header files) it isn't really useful.
-Use your judgment and add it where it helps.
-
 Class overviews
 """""""""""""""
 
@@ -354,33 +336,7 @@ Indent Code Consistently
 
 Okay, in 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.  A few cases are called out here that have common alternatives. The
-intent in saying which way to format things is to increase consistency across
-the LLVM codebase.
-
-Namespaces
-""""""""""
-
-A simple rule: don't indent them. Here are examples of well formatted and
-indented namespaces:
-
-.. code-block:: c++
-  namespace llvm {
-
-  namespace foo {
-  class A;
-  class B;
-  }
-
-  namespace {
-  /// \brief Some local class definition.
-  /// ...
-  class Widget {
-    // ... lots of code here ...
-  };
-  } // End anonymous namespace
-
-  } // End llvm namespace
+Just do it.
 
 Compiler Issues
 ---------------
@@ -1234,46 +1190,10 @@ Namespace Indentation
 
 In general, we strive to reduce indentation wherever 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:
-
-.. code-block:: c++
-
-  namespace llvm {
-    namespace X86 {
-      /// \brief 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 {
-        /// \brief 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,
-
-        /// \brief 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,
-
-        /// \brief 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:
+also because it makes it easier to understand the code. To facilitate this and
+avoid some insanely deep nesting on occasion, don't indent namespaces. If it
+helps readability, feel free to add a comment indicating what namespace is
+being closed by a ``}``.  For example:
 
 .. code-block:: c++
 
@@ -1295,12 +1215,12 @@ closed.  For example:
   } // 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.
+
+Feel free to skip the closing comment when the namespace being closed is
+obvious for any reason. For example, the outer-most namespace in a header file
+is rarely a source of confusion. But namespaces both anonymous and named in
+source files that are being closed half way through the file probably could use
+clarification.
 
 .. _static:
 
@@ -1329,12 +1249,12 @@ good:
 .. code-block:: c++
 
   namespace {
-    class StringSort {
-    ...
-    public:
-      StringSort(...)
-      bool operator<(const char *RHS) const;
-    };
+  class StringSort {
+  ...
+  public:
+    StringSort(...)
+    bool operator<(const char *RHS) const;
+  };
   } // end anonymous namespace
 
   static void runHelper() { 
@@ -1350,6 +1270,7 @@ This is bad:
 .. code-block:: c++
 
   namespace {
+
   class StringSort {
   ...
   public:





More information about the llvm-commits mailing list