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(*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,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(*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 { 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 ` 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 ` 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() {}