diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h --- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h @@ -26,13 +26,16 @@ 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/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp @@ -30,30 +30,6 @@ 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(*Decls.begin()); - return ChildNamespace && !unsupportedNamespace(*ChildNamespace); -} - -template -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 NS::getCleanedNamespaceFrontRange(const SourceManager &SM, const LangOptions &LangOpts) const { @@ -90,19 +66,55 @@ 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()); + } else + 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 (getLangOpts().CPlusPlus20) { + // C++20 support inline nested namespace + bool IsFirstNS = IsChild || !Namespaces.empty(); + return ND.isAnonymousNamespace() || + (ND.isInlineNamespace() && !IsFirstNS) || !ND.attrs().empty(); + } + return ND.isAnonymousNamespace() || ND.isInlineNamespace() || + !ND.attrs().empty(); +} + +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(*Decls.begin()); + return ChildNamespace && !unsupportedNamespace(*ChildNamespace, true); } void ConcatNestedNamespacesCheck::registerMatchers( @@ -137,8 +149,11 @@ 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 +174,15 @@ 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -302,9 +302,9 @@ ``DISABLED_`` in the test suite name. - Improved :doc:`modernize-concat-nested-namespaces - ` to fix incorrect fixes when - using macro between namespace declarations and false positive when using namespace - with attributes. + ` to fix incorrect fixes when + 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 ` 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 --- 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 @@ } } + // in c++20 + namespace n8 { + inline namespace n9 { + void t(); + } + } + Will be modified to: .. code-block:: c++ @@ -47,3 +54,8 @@ } } + // 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 --- 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,14 +1,11 @@ // 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] - namespace n1 {} namespace n2 { @@ -20,12 +17,6 @@ } } // 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 +33,17 @@ 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: :[[@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: :[[@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 +52,7 @@ } } // namespace n12 } // namespace n11 -// CHECK-FIXES: } +// CHECK-FIXES-NORMAL: } namespace n15 { namespace n16 { @@ -75,13 +66,13 @@ 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: :[[@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 +89,36 @@ 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: :[[@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: :[[@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: :[[@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: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] } // namespace n34 -// CHECK-FIXES-EMPTY namespace n36 { void t(); } @@ -142,28 +131,28 @@ #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: :[[@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: :[[@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: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] namespace n43 { #define N43_INNER namespace n44 { @@ -171,12 +160,12 @@ } // 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: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] namespace n45{ #define N45_INNER namespace n46 @@ -189,37 +178,69 @@ } //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: :[[@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: :[[@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: :[[@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: :[[@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: :[[@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: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] namespace /*::*/ comment_colon_2 { namespace comment_colon_2 { void foo() {} @@ -238,3 +259,6 @@ return 0; } + +#include "modernize-concat-nested-namespaces.h" +// CHECK-MESSAGES-NORMAL-DAG: modernize-concat-nested-namespaces.h:1:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]