[clang-tools-extra] 32aaacc - [clang-tidy] support nested inline namespace in c++20 for modernize-concat-nested-namespaces
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 11 12:28:28 PDT 2023
Author: Congcong Cai
Date: 2023-04-11T21:28:11+02:00
New Revision: 32aaacc609e7a0523d498b244e081ac6f3df532b
URL: https://github.com/llvm/llvm-project/commit/32aaacc609e7a0523d498b244e081ac6f3df532b
DIFF: https://github.com/llvm/llvm-project/commit/32aaacc609e7a0523d498b244e081ac6f3df532b.diff
LOG: [clang-tidy] support nested inline namespace in c++20 for modernize-concat-nested-namespaces
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
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize/concat-nested-namespaces.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
index beaa4eeaeb5e7..d24b613015d8e 100644
--- 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 8802c3342446b..941104e8bb143 100644
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -26,13 +26,16 @@ class NS : public llvm::SmallVector<const NamespaceDecl *, 6> {
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 5fb6aa3240517..7eb65d46b9cc1 100644
--- 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 a1fdc7f043dd9..af2378e805223 100644
--- 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 26d28143055f5..fbad0b9884b8a 100644
--- 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() {}
More information about the cfe-commits
mailing list