[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+    const NamespaceContextVec &Namespaces) {
+  std::ostringstream Result;
+  bool First = true;
----------------
Can this be rewritten with `llvm::for_each()` and a `Twine` so that we don't have to use `ostringstream` (which is a big hammer for this).


================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:73
+  diag(Namespaces.front()->getBeginLoc(),
+       "Nested namespaces can be concatenated", DiagnosticIDs::Warning)
+      << FixItHint::CreateReplacement(FrontReplacement,
----------------
Diagnostics should not start with a capital letter.


================
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+  using NamespaceContextVec = llvm::SmallVector<const NamespaceDecl *, 6>;
+
----------------
Why 6?


================
Comment at: docs/clang-tidy/checks/list.rst:13
    abseil-str-cat-append
+   abseil-string-find-startswith
    android-cloexec-accept
----------------
This is an unrelated change -- feel free to make a separate commit that fixes this (no review needed).


https://reviews.llvm.org/D52136