HerrCai0907 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
HerrCai0907 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
remove each namespace except the last non-nest namespace,
replace the last non-nest namespace with the new name.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147194
Files:
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -156,6 +156,20 @@
} // namespace n41
// CHECK-FIXES: }
+
+namespace n43 {
+// CHECK-MESSAGES-DAG: :[[@LINE-1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+#define N43_INNER
+namespace n44 {
+void foo() {}
+} // namespace n44
+#undef N43_INNER
+} // namespace n43
+// CHECK-FIXES: #define N43_INNER
+// CHECK-FIXES: namespace n43::n44 {
+// CHECK-FIXES: }
+// CHECK-FIXES: #undef N43_INNER
+
int main() {
n26::n27::n28::n29::n30::t();
#ifdef IEXIST
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h
@@ -5,4 +5,4 @@
} // namespace nn2
} // namespace nn1
// CHECK-FIXES: void t();
-// CHECK-FIXES-NEXT: } // namespace nn1
+// CHECK-FIXES-NEXT: } // namespace nn2
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.h
@@ -29,8 +29,8 @@
using NamespaceContextVec = llvm::SmallVector<const NamespaceDecl *, 6>;
using NamespaceString = llvm::SmallString<40>;
- void reportDiagnostic(const SourceRange &FrontReplacement,
- const SourceRange &BackReplacement);
+ void reportDiagnostic(const SourceManager &Sources,
+ const LangOptions &LangOpts);
NamespaceString concatNamespaces();
NamespaceContextVec Namespaces;
};
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -9,8 +9,10 @@
#include "ConcatNestedNamespacesCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Lexer.h"
#include <algorithm>
+#include <optional>
namespace clang::tidy::modernize {
@@ -20,6 +22,13 @@
Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
}
+static StringRef getRawStringRef(const SourceRange &Range,
+ const SourceManager &Sources,
+ const LangOptions &LangOpts) {
+ CharSourceRange TextRange = Lexer::getAsCharRange(Range, Sources, LangOpts);
+ return Lexer::getSourceText(TextRange, Sources, LangOpts);
+}
+
static bool anonymousOrInlineNamespace(const NamespaceDecl &ND) {
return ND.isAnonymousNamespace() || ND.isInlineNamespace();
}
@@ -38,11 +47,66 @@
const SourceManager &Sources,
const LangOptions &LangOpts) {
// FIXME: This logic breaks when there is a comment with ':'s in the middle.
- CharSourceRange TextRange =
- Lexer::getAsCharRange(ReplacementRange, Sources, LangOpts);
- StringRef CurrentNamespacesText =
- Lexer::getSourceText(TextRange, Sources, LangOpts);
- return CurrentNamespacesText.count(':') == (NumCandidates - 1) * 2;
+ return getRawStringRef(ReplacementRange, Sources, LangOpts).count(':') ==
+ (NumCandidates - 1) * 2;
+}
+
+static std::optional<Token> findNextToken(SourceLocation Loc,
+ const SourceManager &SM,
+ const LangOptions &LangOpts,
+ bool WithComment) {
+ if (Loc.isMacroID())
+ return std::nullopt;
+ Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts);
+ // Break down the source location.
+ std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
+ bool InvalidTemp = false;
+ StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
+ if (InvalidTemp)
+ return std::nullopt;
+ // Lex from the start of the given location.
+ Lexer L(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
+ File.data() + LocInfo.second, File.end());
+ L.SetCommentRetentionState(WithComment);
+ // Find the token.
+ Token Tok;
+ L.LexFromRawLexer(Tok);
+ return Tok;
+}
+
+static std::optional<SourceRange>
+getCleanedNamespaceFrontRange(const NamespaceDecl *ND, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ // Front from namespace tp '{'
+ std::optional<Token> Tok =
+ findNextToken(ND->getLocation(), SM, LangOpts, false);
+ if (!Tok)
+ return std::nullopt;
+ while (Tok->getKind() != tok::TokenKind::l_brace) {
+ Tok = findNextToken(Tok->getEndLoc(), SM, LangOpts, false);
+ if (!Tok)
+ return std::nullopt;
+ }
+ return SourceRange{ND->getBeginLoc(), Tok->getEndLoc()};
+}
+
+static SourceRange getCleanedNamespaceBackRange(const NamespaceDecl *ND,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ // Back from '}' to conditional '// namespace xxx'
+ const SourceRange DefaultSourceRange =
+ SourceRange{ND->getRBraceLoc(), ND->getRBraceLoc()};
+ SourceLocation Loc = ND->getRBraceLoc();
+ std::optional<Token> Tok = findNextToken(Loc, SM, LangOpts, true);
+ if (!Tok)
+ return DefaultSourceRange;
+ if (Tok->getKind() != tok::TokenKind::comment)
+ return DefaultSourceRange;
+ SourceRange TokRange = SourceRange{Tok->getLocation(), Tok->getEndLoc()};
+ StringRef TokText = getRawStringRef(TokRange, SM, LangOpts);
+ if (TokText != "// namespace " + ND->getNameAsString())
+ return DefaultSourceRange;
+ return SourceRange{ND->getRBraceLoc(), Tok->getEndLoc()};
}
ConcatNestedNamespacesCheck::NamespaceString
@@ -65,11 +129,44 @@
}
void ConcatNestedNamespacesCheck::reportDiagnostic(
- const SourceRange &FrontReplacement, const SourceRange &BackReplacement) {
- diag(Namespaces.front()->getBeginLoc(),
- "nested namespaces can be concatenated", DiagnosticIDs::Warning)
- << FixItHint::CreateReplacement(FrontReplacement, concatNamespaces())
- << FixItHint::CreateReplacement(BackReplacement, "}");
+ const SourceManager &SM, const LangOptions &LangOpts) {
+ DiagnosticBuilder DB =
+ diag(Namespaces.front()->getBeginLoc(),
+ "nested namespaces can be concatenated", DiagnosticIDs::Warning);
+
+ SmallVector<SourceRange, 6> Fronts;
+ Fronts.reserve(Namespaces.size() - 1U);
+ SmallVector<SourceRange, 6> Backs;
+ Backs.reserve(Namespaces.size());
+
+ NamespaceDecl const *ND; // ND is the last non-nest namespace
+
+ for (size_t Index = 0; Index != Namespaces.size(); Index++) {
+ if (Namespaces[Index]->isNested()) {
+ continue;
+ }
+ ND = Namespaces[Index];
+ std::optional<SourceRange> SR =
+ getCleanedNamespaceFrontRange(ND, SM, LangOpts);
+ if (!SR.has_value()) {
+ return;
+ }
+ Fronts.push_back(SR.value());
+ Backs.push_back(getCleanedNamespaceBackRange(ND, SM, LangOpts));
+ }
+ // the last one should be handled specially
+ Fronts.pop_back();
+ Backs.pop_back();
+
+ for (SourceRange const &Front : Fronts) {
+ DB << FixItHint::CreateRemoval(Front);
+ }
+ for (SourceRange const &Back : Backs) {
+ DB << FixItHint::CreateRemoval(Back);
+ }
+
+ DB << FixItHint::CreateReplacement(
+ SourceRange{ND->getBeginLoc(), ND->getLocation()}, concatNamespaces());
}
void ConcatNestedNamespacesCheck::check(
@@ -90,12 +187,10 @@
SourceRange FrontReplacement(Namespaces.front()->getBeginLoc(),
Namespaces.back()->getLocation());
- SourceRange BackReplacement(Namespaces.back()->getRBraceLoc(),
- Namespaces.front()->getRBraceLoc());
if (!alreadyConcatenated(Namespaces.size(), FrontReplacement, Sources,
getLangOpts()))
- reportDiagnostic(FrontReplacement, BackReplacement);
+ reportDiagnostic(Sources, getLangOpts());
Namespaces.clear();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
