[clang-tidy] support nested inline namespace in c++20 for modernize-concat-nested...

Fixed https://github.com/llvm/llvm-project/issues/56022
c++20 support namespace like `namespace a::inline b {}`.
If an inline namespace is not the first, it can be concatened.

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D147946

diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
index beaa4ee..d24b613 100644 (file)
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -30,30 +30,6 @@ static StringRef getRawStringRef(const SourceRange &Range,
   return Lexer::getSourceText(TextRange, Sources, LangOpts);
 }
 
-static bool unsupportedNamespace(const NamespaceDecl &ND) {
-  return ND.isAnonymousNamespace() || ND.isInlineNamespace() ||
-         !ND.attrs().empty();
-}
-
-static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
-  NamespaceDecl::decl_range Decls = ND.decls();
-  if (std::distance(Decls.begin(), Decls.end()) != 1)
-    return false;
-
-  const auto *ChildNamespace = dyn_cast<const NamespaceDecl>(*Decls.begin());
-  return ChildNamespace && !unsupportedNamespace(*ChildNamespace);
-}
-
-template <class R, class F>
-static void concatNamespace(NamespaceName &ConcatNameSpace, R &&Range,
-                            F &&Stringify) {
-  for (auto const &V : Range) {
-    ConcatNameSpace.append(Stringify(V));
-    if (V != Range.back())
-      ConcatNameSpace.append("::");
-  }
-}
-
 std::optional<SourceRange>
 NS::getCleanedNamespaceFrontRange(const SourceManager &SM,
                                   const LangOptions &LangOpts) const {
@@ -90,19 +66,53 @@ SourceRange NS::getNamespaceBackRange(const SourceManager &SM,
     return getDefaultNamespaceBackRange();
   SourceRange TokRange = SourceRange{Tok->getLocation(), Tok->getEndLoc()};
   StringRef TokText = getRawStringRef(TokRange, SM, LangOpts);
-  std::string CloseComment = ("namespace " + getName()).str();
+  NamespaceName CloseComment{"namespace "};
+  appendCloseComment(CloseComment);
   // current fix hint in readability/NamespaceCommentCheck.cpp use single line
   // comment
-  if (TokText != "// " + CloseComment && TokText != "//" + CloseComment)
+  constexpr size_t L = sizeof("//") - 1U;
+  if (TokText.take_front(L) == "//" &&
+      TokText.drop_front(L).trim() != CloseComment)
     return getDefaultNamespaceBackRange();
   return SourceRange{front()->getRBraceLoc(), Tok->getEndLoc()};
 }
 
-NamespaceName NS::getName() const {
-  NamespaceName Name{};
-  concatNamespace(Name, *this,
-                  [](const NamespaceDecl *ND) { return ND->getName(); });
-  return Name;
+void NS::appendName(NamespaceName &Str) const {
+  for (const NamespaceDecl *ND : *this) {
+    if (ND->isInlineNamespace())
+      Str.append("inline ");
+    Str.append(ND->getName());
+    if (ND != back())
+      Str.append("::");
+  }
+}
+void NS::appendCloseComment(NamespaceName &Str) const {
+  if (size() == 1)
+    Str.append(back()->getName());
+  else
+    appendName(Str);
+}
+
+bool ConcatNestedNamespacesCheck::unsupportedNamespace(const NamespaceDecl &ND,
+                                                       bool IsChild) const {
+  if (ND.isAnonymousNamespace() || !ND.attrs().empty())
+    return true;
+  if (getLangOpts().CPlusPlus20) {
+    // C++20 support inline nested namespace
+    bool IsFirstNS = IsChild || !Namespaces.empty();
+    return ND.isInlineNamespace() && !IsFirstNS;
+  }
+  return ND.isInlineNamespace();
+}
+
+bool ConcatNestedNamespacesCheck::singleNamedNamespaceChild(
+    const NamespaceDecl &ND) const {
+  NamespaceDecl::decl_range Decls = ND.decls();
+  if (std::distance(Decls.begin(), Decls.end()) != 1)
+    return false;
+
+  const auto *ChildNamespace = dyn_cast<const NamespaceDecl>(*Decls.begin());
+  return ChildNamespace && !unsupportedNamespace(*ChildNamespace, true);
 }
 
 void ConcatNestedNamespacesCheck::registerMatchers(
@@ -137,8 +147,11 @@ void ConcatNestedNamespacesCheck::reportDiagnostic(
   SourceRange LastRBrace = Backs.pop_back_val();
 
   NamespaceName ConcatNameSpace{"namespace "};
-  concatNamespace(ConcatNameSpace, Namespaces,
-                  [](const NS &NS) { return NS.getName(); });
+  for (const NS &NS : Namespaces) {
+    NS.appendName(ConcatNameSpace);
+    if (&NS != &Namespaces.back()) // compare address directly
+      ConcatNameSpace.append("::");
+  }
 
   for (SourceRange const &Front : Fronts)
     DB << FixItHint::CreateRemoval(Front);
@@ -159,12 +172,15 @@ void ConcatNestedNamespacesCheck::check(
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
     return;
 
-  if (unsupportedNamespace(ND))
+  if (unsupportedNamespace(ND, false))
     return;
 
   if (!ND.isNested())
     Namespaces.push_back(NS{});
-  Namespaces.back().push_back(&ND);
+  if (!Namespaces.empty())
+    // Otherwise it will crash with invalid input like `inline namespace
+    // a::b::c`.
+    Namespaces.back().push_back(&ND);
 
   if (singleNamedNamespaceChild(ND))
     return;

diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
index 8802c33..941104e 100644 (file)
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -26,13 +26,16 @@ public:
   SourceRange getNamespaceBackRange(const SourceManager &SM,
                                     const LangOptions &LangOpts) const;
   SourceRange getDefaultNamespaceBackRange() const;
-  NamespaceName getName() const;
+  void appendName(NamespaceName &Str) const;
+  void appendCloseComment(NamespaceName &Str) const;
 };
 
 class ConcatNestedNamespacesCheck : public ClangTidyCheck {
 public:
   ConcatNestedNamespacesCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
+  bool unsupportedNamespace(const NamespaceDecl &ND, bool IsChild) const;
+  bool singleNamedNamespaceChild(const NamespaceDecl &ND) const;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus17;
   }

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5fb6aa3..7eb65d4 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -314,8 +314,8 @@ Changes in existing checks
 
 - Improved :doc:`modernize-concat-nested-namespaces
   <clang-tidy/checks/modernize/concat-nested-namespaces>` to fix incorrect fixes when
-  using macro between namespace declarations and false positive when using namespace
-  with attributes.
+  using macro between namespace declarations, to fix false positive when using namespace
+  with attributes and to support nested inline namespace introduced in c++20.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   <clang-tidy/checks/performance/no-automatic-move>` when warning would be

diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/concat-nested-namespaces.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/concat-nested-namespaces.rst
index a1fdc7f..af2378e 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/concat-nested-namespaces.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/concat-nested-namespaces.rst
@@ -30,6 +30,13 @@ For example:
   }
   }
 
+  // in c++20
+  namespace n8 {
+  inline namespace n9 {
+  void t();
+  }
+  }
+
 Will be modified to:
 
 .. code-block:: c++
@@ -47,3 +54,8 @@ Will be modified to:
   }
   }
 
+  // in c++20
+  namespace n8::inline n9 {
+  void t();
+  }
+

diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
index 26d2814..fbad0b9 100644 (file)
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -1,13 +1,13 @@
 // RUN: cp %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %T/modernize-concat-nested-namespaces.h
-// RUN: %check_clang_tidy -std=c++17 %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %T
+// RUN: %check_clang_tidy -std=c++17 -check-suffix=NORMAL %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %T
 // RUN: FileCheck -input-file=%T/modernize-concat-nested-namespaces.h %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h -check-prefix=CHECK-FIXES
 // Restore header file and re-run with c++20:
 // RUN: cp %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %T/modernize-concat-nested-namespaces.h
-// RUN: %check_clang_tidy -std=c++20 %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %T
+// RUN: %check_clang_tidy -std=c++20 -check-suffixes=NORMAL,CPP20 %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %T
 // RUN: FileCheck -input-file=%T/modernize-concat-nested-namespaces.h %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h -check-prefix=CHECK-FIXES
 
 #include "modernize-concat-nested-namespaces.h"
-// CHECK-MESSAGES-DAG: modernize-concat-nested-namespaces.h:1:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: modernize-concat-nested-namespaces.h:1:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 
 namespace n1 {}
 
@@ -20,12 +20,6 @@ void t();
 }
 } // namespace n2
 
-namespace n5 {
-inline namespace inline_ns {
-void t();
-} // namespace inline_ns
-} // namespace n5
-
 namespace n6 {
 namespace [[deprecated]] attr_ns {
 void t();
@@ -42,17 +36,17 @@ void t();
 
 namespace n9 {
 namespace n10 {
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n9::n10
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n9::n10
 void t();
 } // namespace n10
 } // namespace n9
-// CHECK-FIXES: }
+// CHECK-FIXES-NORMAL: }
 
 namespace n11 {
 namespace n12 {
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n11::n12
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n11::n12
 namespace n13 {
 void t();
 }
@@ -61,7 +55,7 @@ void t();
 }
 } // namespace n12
 } // namespace n11
-// CHECK-FIXES: }
+// CHECK-FIXES-NORMAL: }
 
 namespace n15 {
 namespace n16 {
@@ -75,13 +69,13 @@ void t();
 namespace n18 {
 namespace n19 {
 namespace n20 {
-// CHECK-MESSAGES-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n18::n19::n20
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n18::n19::n20
 void t();
 } // namespace n20
 } // namespace n19
 } // namespace n18
-// CHECK-FIXES: }
+// CHECK-FIXES-NORMAL: }
 
 namespace n21 {
 void t();
@@ -98,38 +92,36 @@ namespace n23 {
 namespace {
 namespace n24 {
 namespace n25 {
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n24::n25
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n24::n25
 void t();
 } // namespace n25
 } // namespace n24
-// CHECK-FIXES: }
+// CHECK-FIXES-NORMAL: }
 } // namespace
 } // namespace n23
 
 namespace n26::n27 {
 namespace n28 {
 namespace n29::n30 {
-// CHECK-MESSAGES-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n26::n27::n28::n29::n30 {
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n26::n27::n28::n29::n30 {
 void t() {}
 } // namespace n29::n30
 } // namespace n28
 } // namespace n26::n27
-// CHECK-FIXES: }
+// CHECK-FIXES-NORMAL: }
 
 namespace n31 {
 namespace n32 {}
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 } // namespace n31
-// CHECK-FIXES-EMPTY
 
 namespace n33 {
 namespace n34 {
 namespace n35 {}
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 } // namespace n34
-// CHECK-FIXES-EMPTY
 namespace n36 {
 void t();
 }
@@ -142,28 +134,28 @@ void t();
 #define IEXIST
 namespace n39 {
 namespace n40 {
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n39::n40
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n39::n40
 #ifdef IEXIST
 void t() {}
 #endif
 } // namespace n40
 } // namespace n39
-// CHECK-FIXES: } // namespace n39::n40
+// CHECK-FIXES-NORMAL: } // namespace n39::n40
 
 namespace n41 {
 namespace n42 {
-// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n41::n42
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: namespace n41::n42
 #ifdef IDONTEXIST
 void t() {}
 #endif
 } // namespace n42
 } // namespace n41
-// CHECK-FIXES: } // namespace n41::n42
+// CHECK-FIXES-NORMAL: } // namespace n41::n42
 
 
-// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 namespace n43 {
 #define N43_INNER
 namespace n44 {
@@ -171,12 +163,12 @@ void foo() {}
 } // namespace n44
 #undef N43_INNER
 } // namespace n43
-// CHECK-FIXES: #define N43_INNER
-// CHECK-FIXES: namespace n43::n44 {
-// CHECK-FIXES: } // namespace n43::n44
-// CHECK-FIXES: #undef N43_INNER
+// CHECK-FIXES-NORMAL: #define N43_INNER
+// CHECK-FIXES-NORMAL: namespace n43::n44 {
+// CHECK-FIXES-NORMAL: } // namespace n43::n44
+// CHECK-FIXES-NORMAL: #undef N43_INNER
 
-// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 namespace n45{
 #define N45_INNER
 namespace n46
@@ -189,37 +181,69 @@ void foo() {}
 } //namespace n46
 #undef N45_INNER
 } //namespace n45
-// CHECK-FIXES: #define N45_INNER
-// CHECK-FIXES: #pragma clang diagnostic push
-// CHECK-FIXES: namespace n45::n46::n47 {
-// CHECK-FIXES: } // namespace n45::n46::n47
-// CHECK-FIXES: #pragma clang diagnostic pop
-// CHECK-FIXES: #undef N45_INNER
-
-// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES-NORMAL: #define N45_INNER
+// CHECK-FIXES-NORMAL: #pragma clang diagnostic push
+// CHECK-FIXES-NORMAL: namespace n45::n46::n47 {
+// CHECK-FIXES-NORMAL: } // namespace n45::n46::n47
+// CHECK-FIXES-NORMAL: #pragma clang diagnostic pop
+// CHECK-FIXES-NORMAL: #undef N45_INNER
+
+inline namespace n48 {
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+namespace n49 {
+namespace n50 {
+// CHECK-FIXES-NORMAL: namespace n49::n50 {
+void foo() {}
+}
+}
+}
+
+// CHECK-MESSAGES-CPP20-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+namespace n51 {
+inline namespace n52 {
+namespace n53 {
+// CHECK-FIXES-CPP20: namespace n51::inline n52::n53 {
+void foo() {}
+}
+}
+}
+
+#if __cplusplus >= 202002L
+// CHECK-MESSAGES-CPP20-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+namespace n54 {
+namespace n55::inline n56::n57 {
+namespace n58 {
+// CHECK-FIXES-CPP20: namespace n54::n55::inline n56::n57::n58 {
+void foo() {}
+}
+}
+}
+#endif
+
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 namespace avoid_add_close_comment {
 namespace inner {
 void foo() {}
 }
 }
-// CHECK-FIXES: namespace avoid_add_close_comment::inner {
-// CHECK-FIXES-NOT: } // namespace avoid_add_close_comment::inner
+// CHECK-FIXES-NORMAL: namespace avoid_add_close_comment::inner {
+// CHECK-FIXES-NORMAL-NOT: } // namespace avoid_add_close_comment::inner
 
-// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 namespace avoid_change_close_comment {
 namespace inner {
 void foo() {}
 } // namespace inner and other comments
 } // namespace avoid_change_close_comment and other comments
-// CHECK-FIXES: namespace avoid_change_close_comment::inner {
-// CHECK-FIXES-NOT: } // namespace avoid_add_close_comment::inner
+// CHECK-FIXES-NORMAL: namespace avoid_change_close_comment::inner {
+// CHECK-FIXES-NORMAL-NOT: } // namespace avoid_add_close_comment::inner
 
 namespace /*::*/ comment_colon_1 {
 void foo() {}
 } // namespace comment_colon_1
-// CHECK-FIXES: namespace /*::*/ comment_colon_1 {
+// CHECK-FIXES-NORMAL: namespace /*::*/ comment_colon_1 {
 
-// CHECK-MESSAGES-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-MESSAGES-NORMAL-DAG: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
 namespace /*::*/ comment_colon_2 {
 namespace comment_colon_2 {
 void foo() {}