[PATCH] D74515: Add coding standard recommending use of qualifiers in cpp files

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: MaskRay, rjmccall, jlebar, dblaikie.
Herald added a project: LLVM.

There is prior art for this in the code base itself, and a recent
example of this here: c45f8d49897f

This came up in discussion on this review where @maskray was going the
opposite direction:

  https://reviews.llvm.org/D68772

Given that there is disagreement, we should make a choice and document
it.

My wording could probably use improvement, but I figured I'd send this
out anyway to see if people are receptive to the guideline.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74515

Files:
  llvm/docs/CodingStandards.rst


Index: llvm/docs/CodingStandards.rst
===================================================================
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -762,6 +762,60 @@
     It's okay to put extra implementation methods in a public class itself. Just
     make them private (or protected) and all is well.
 
+Open Namespaces in Header Files, Not Source Files
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+To declare things in a namespace in a header file, open a namespace in the usual
+way:
+
+.. code-block:: c++
+
+  // Foo.h
+  namespace llvm {
+  class Foo {
+  public:
+    int foo();
+    // ...
+  };
+  int bar(Foo *p);
+  }
+
+When providing implementations in a source file for functions or methods
+declared in a header file, do not open namespace blocks in the source file.
+Instead, use namespace qualifiers to help ensure that your definition matches an
+existing declaration. Do this:
+
+.. code-block:: c++
+
+  // Foo.cpp
+  #include "Foo.h"
+
+  using namespace llvm;
+
+  int Foo::foo() { /* ... */ }
+
+  int llvm::bar(Foo *p) {
+    // ...
+  }
+
+If every definition in the implementation file uses a previously declared
+qualifier, it helps to catch bugs where the definition does not match the
+declaration. In C++, this would declare a new overload of ``llvm::bar``, for
+example:
+
+.. code-block:: c++
+
+  // Foo.cpp
+  #include "Foo.h"
+
+  namespace llvm {
+  int bar(Bar *p) { // Note "Bar" does not match "Foo" above.
+  }
+  } // end namespace llvm
+
+It is more likely that the programmer intended to implement ``llvm::bar(Foo*)``
+in this case, but has not yet updated the prototype.
+
 .. _early exits:
 
 Use Early Exits and ``continue`` to Simplify Code


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74515.244298.patch
Type: text/x-patch
Size: 1726 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200212/7cc0fc0b/attachment.bin>