[PATCH] D81075: [ASTMatchers] Fix crash with HasName and HasAnyName

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: klimek, sbenza.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes crashes caused by matching fully qualified RecordDecls inside Function 
bodies.
See https://bugs.llvm.org/show_bug.cgi?id=43639.
I'm still unsure if this is the desired behaviour though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81075

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/Decl.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1665,7 +1665,7 @@
   StringRef code =
       "namespace a { void F(int a) { struct S { int m; }; int i; } }";
   EXPECT_TRUE(matches(code, varDecl(hasName("i"))));
-  EXPECT_FALSE(matches(code, varDecl(hasName("F()::i"))));
+  EXPECT_FALSE(matches(code, varDecl(hasName("F(int)::i"))));
 
   EXPECT_TRUE(matches(code, fieldDecl(hasName("m"))));
   EXPECT_TRUE(matches(code, fieldDecl(hasName("S::m"))));
@@ -1674,6 +1674,34 @@
   EXPECT_TRUE(matches(code, fieldDecl(hasName("::a::F(int)::S::m"))));
 }
 
+TEST(Matcher, HasNameSupportsFunctionScopeRecordDecl) {
+  StringRef FuncCode = R"cc(
+    void A() {
+      struct B {};
+    })cc";
+  EXPECT_TRUE(matches(FuncCode, cxxRecordDecl(hasName("B"))));
+  EXPECT_TRUE(matches(FuncCode, cxxRecordDecl(hasName("A()::B"))));
+  EXPECT_TRUE(matches(FuncCode, cxxRecordDecl(hasName("::A()::B"))));
+  EXPECT_TRUE(notMatches(FuncCode, cxxRecordDecl(hasName("::B"))));
+
+  StringRef MethodCode = R"cc(
+    struct A {
+      void B() {
+        struct C {};
+      }
+    };)cc";
+
+  EXPECT_TRUE(matches(MethodCode, cxxRecordDecl(hasName("C"))));
+  EXPECT_TRUE(matches(MethodCode, cxxRecordDecl(hasName("B()::C"))));
+  EXPECT_TRUE(
+      matches(MethodCode, cxxRecordDecl(hasName("A::B()::C"))));
+  EXPECT_TRUE(
+      matches(MethodCode, cxxRecordDecl(hasName("::A::B()::C"))));
+  EXPECT_TRUE(notMatches(MethodCode, cxxRecordDecl(hasName("::C"))));
+  EXPECT_TRUE(
+      notMatches(MethodCode, cxxRecordDecl(hasName("::B()::C"))));
+}
+
 TEST(Matcher, HasNameQualifiedSupportsLinkage) {
   // https://bugs.llvm.org/show_bug.cgi?id=42193
   StringRef code = R"cpp(namespace foo { extern "C" void test(); })cpp";
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -571,7 +571,7 @@
   // We are allowed to skip anonymous and inline namespaces if they don't match.
   const DeclContext *Ctx = Node.getDeclContext();
 
-  if (Ctx->isFunctionOrMethod())
+  if (Ctx->isFunctionOrMethod() && !isa<RecordDecl>(Node))
     return Patterns.foundMatch(/*AllowFullyQualified=*/false);
 
   for (; Ctx; Ctx = Ctx->getParent()) {
@@ -616,13 +616,10 @@
     llvm::SmallString<128> NodeName = StringRef("::");
     llvm::raw_svector_ostream OS(NodeName);
 
-    if (SkipUnwritten) {
-      PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy();
-      Policy.SuppressUnwrittenScope = true;
-      Node.printQualifiedName(OS, Policy);
-    } else {
-      Node.printQualifiedName(OS);
-    }
+    PrintingPolicy Policy = Node.getASTContext().getPrintingPolicy();
+    Policy.PrintFunctionQualifiedRecords = true;
+    Policy.SuppressUnwrittenScope = SkipUnwritten;
+    Node.printQualifiedName(OS, Policy);
 
     const StringRef FullName = OS.str();
 
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1540,7 +1540,8 @@
 
 void NamedDecl::printQualifiedName(raw_ostream &OS,
                                    const PrintingPolicy &P) const {
-  if (getDeclContext()->isFunctionOrMethod()) {
+  if (getDeclContext()->isFunctionOrMethod() &&
+      !(P.PrintFunctionQualifiedRecords && isa<RecordDecl>(this))) {
     // We do not print '(anonymous)' for function parameters without name.
     printName(OS);
     return;
@@ -1583,7 +1584,8 @@
       Ctx = CI;
   }
 
-  if (Ctx->isFunctionOrMethod())
+  if (Ctx->isFunctionOrMethod() &&
+      !(P.PrintFunctionQualifiedRecords && isa<RecordDecl>(this)))
     return;
 
   using ContextsTy = SmallVector<const DeclContext *, 8>;
Index: clang/include/clang/AST/PrettyPrinter.h
===================================================================
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -63,7 +63,8 @@
         MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
         MSVCFormatting(false), ConstantsAsWritten(false),
         SuppressImplicitBase(false), FullyQualifiedName(false),
-        PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true) {}
+        PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true),
+        PrintFunctionQualifiedRecords(false) {}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -249,6 +250,18 @@
   /// invalid C++ code.
   unsigned PrintInjectedClassNameWithArguments : 1;
 
+  /// Whether to print FunctionDecl surrounding a RecordDecl.
+  ///
+  /// \code
+  /// void Foo(int Bar) {
+  ///   struct Baz;
+  /// }
+  /// \endcode
+  ///
+  /// When true, Bar will be printed as:
+  /// Foo(int)::Baz
+  unsigned PrintFunctionQualifiedRecords : 1;
+
   /// Callbacks to use to allow the behavior of printing to be customized.
   const PrintingCallbacks *Callbacks = nullptr;
 };

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