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

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked an inline comment as done.
rnk 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;
----------------
MaskRay wrote:
> 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......
I don't think we have rules because it really hasn't come up. LLVM just doesn't have that many namespaces, just llvm::, clang::, and some small per-library namespaces in that area. So we only ever have two or so using namespace decls in most source code. I don't feel like the header guidelines define an ordering between the project headers. For example, llvm includes and clang includes are part of the same block, sorted alphabetically.

In any case, as written now, it is closer to a straight revert. Should we do that and decide on namespace ordering rules another time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74882