[PATCH] D141787: [clang-tidy] fix a false positive of `modernize-concat-nested-namespaces`

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 489350.
v1nh1shungry added a comment.

add a test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141787/new/

https://reviews.llvm.org/D141787

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  
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,29 @@
 } // namespace n41
 // CHECK-FIXES: }
 
+namespace n43 {
+#define FOO
+namespace n44 {
+} // namespace n44
+} // namespace n43
+
+namespace n45 {
+namespace n46 {
+} // namespace n46
+#define BAR
+} // namespace n45
+
+namespace n47 {
+namespace n48 {
+// CHECK-MESSAGES-DAG: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n47::n48
+#define FOOBAR
+namespace n49 {
+} // namespace n49
+} // namespace n48
+} // namespace n47
+// CHECK-FIXES: }
+
 int main() {
   n26::n27::n28::n29::n30::t();
 #ifdef IEXIST
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
@@ -24,13 +24,37 @@
   return ND.isAnonymousNamespace() || ND.isInlineNamespace();
 }
 
-static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
+static bool hasPPDirective(SourceRange Range, const SourceManager &SM,
+                           const LangOptions &LangOpts) {
+  CharSourceRange CharRange =
+      Lexer::makeFileCharRange({Range, true}, SM, LangOpts);
+  StringRef BetweenText = Lexer::getSourceText(CharRange, SM, LangOpts);
+  std::string Buffer{BetweenText};
+  Lexer Lex(Range.getBegin(), LangOpts, Buffer.c_str(), Buffer.c_str(),
+            Buffer.c_str() + Buffer.size());
+  Token Tok;
+  while (!Lex.LexFromRawLexer(Tok)) {
+    if (Tok.getKind() == tok::hash)
+      return true;
+  }
+  return false;
+}
+
+static bool singleNamedNamespaceChild(const NamespaceDecl &ND,
+                                      const SourceManager &SM,
+                                      const LangOptions &LangOpts) {
   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 && !anonymousOrInlineNamespace(*ChildNamespace);
+  if (!ChildNamespace || anonymousOrInlineNamespace(*ChildNamespace))
+    return false;
+
+  return !hasPPDirective({ND.getBeginLoc(), ChildNamespace->getBeginLoc()}, SM,
+                         LangOpts) &&
+         !hasPPDirective({ChildNamespace->getRBraceLoc(), ND.getRBraceLoc()},
+                         SM, LangOpts);
 }
 
 static bool alreadyConcatenated(std::size_t NumCandidates,
@@ -76,6 +100,7 @@
     const ast_matchers::MatchFinder::MatchResult &Result) {
   const NamespaceDecl &ND = *Result.Nodes.getNodeAs<NamespaceDecl>("namespace");
   const SourceManager &Sources = *Result.SourceManager;
+  const LangOptions &LangOpts = getLangOpts();
 
   if (!locationsInSameFile(Sources, ND.getBeginLoc(), ND.getRBraceLoc()))
     return;
@@ -85,7 +110,7 @@
 
   Namespaces.push_back(&ND);
 
-  if (singleNamedNamespaceChild(ND))
+  if (singleNamedNamespaceChild(ND, Sources, LangOpts))
     return;
 
   SourceRange FrontReplacement(Namespaces.front()->getBeginLoc(),
@@ -94,7 +119,7 @@
                               Namespaces.front()->getRBraceLoc());
 
   if (!alreadyConcatenated(Namespaces.size(), FrontReplacement, Sources,
-                           getLangOpts()))
+                           LangOpts))
     reportDiagnostic(FrontReplacement, BackReplacement);
 
   Namespaces.clear();

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits