[PATCH] D74882: Remove namespace lld { namespace coff { from COFF LLD cpp files

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 20 15:34:52 PST 2020


MaskRay added inline comments.


================
Comment at: lld/COFF/DebugTypes.cpp:23
 using namespace llvm::codeview;
-
-namespace lld {
-namespace coff {
+using namespace lld;
+using namespace lld::coff;
----------------
rnk wrote:
> MaskRay wrote:
> > Other files `using namespace lld` before `llvm`.
> That's true. Actually, I'm not sure I like it. I started just feeding these blocks to vim `:sort`, but now it makes them like this:
> ```
> using namespace lld::coff;
> using namespace lld;
> using namespace llvm::codeview;
> using namespace llvm;
> ```
> This seems kind of wrong, it's better to go most general to most specific. At the very least, we should use lld before lld::coff. By the same reasoning, we could put llvm before lld, since it has general things like ADT, StringRef, etc.
For `#include`s, we follow this order:

* Main Module Header
* Local/Private Headers
* LLVM project/subproject headers (clang/..., lldb/..., llvm/..., etc)
* System `#include`s

lld headers are included before LLVM headers. Now it becomes unclear to me why for `using` directives we follow a different convention......


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74882/new/

https://reviews.llvm.org/D74882





More information about the llvm-commits mailing list