[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet.
Herald added a project: clang.

RecursiveASTVisitor was visiting implcit constructor initializers. This caused 
semantic highlighting in clangd to emit error logs. Fixes this by checking if 
the constructor is written or if the visitor should visit implicit decls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65735

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp


Index: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
===================================================================
--- clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
@@ -22,11 +22,19 @@
 
   bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+    if (Init->getSourceLocation().isInvalid())
+      InvalidLocsVisited = true;
+    Match("initializer", Init->getSourceLocation());
+    return true;
+  }
+
   bool VisitInitListExpr(InitListExpr *ILE) {
     Match(ILE->isSemanticForm() ? "semantic" : "syntactic", 
ILE->getBeginLoc());
     return true;
   }
 
+  bool InvalidLocsVisited = false;
 private:
   bool VisitImplicitCode;
 };
@@ -49,4 +57,22 @@
                               InitListExprPreOrderVisitor::Lang_C));
 }
 
+TEST(RecursiveASTVisitor, CXXCtorInitializerVisitNoImplicit) {
+  std::vector<bool> Config{true, false};
+  for (bool VisitImplCode : Config) {
+    InitListExprPreOrderVisitor Visitor(VisitImplCode);
+    Visitor.ExpectMatch("initializer", 7, 17);
+    EXPECT_TRUE(Visitor.runOver(R"cpp(
+        class A {};
+        class B : public A {
+          B() {};
+        };
+        class C : public A {
+          C() : A() {}
+        };
+      )cpp",
+                                InitListExprPreOrderVisitor::Lang_CXX));
+    EXPECT_EQ(Visitor.InvalidLocsVisited, VisitImplCode);
+  }
+}
 } // end anonymous namespace
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2023,7 +2023,8 @@
   if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
     // Constructor initializers.
     for (auto *I : Ctor->inits()) {
-      TRY_TO(TraverseConstructorInitializer(I));
+      if (I->isWritten() || getDerived().shouldVisitImplicitCode())
+        TRY_TO(TraverseConstructorInitializer(I));
     }
   }
 



Index: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
===================================================================
--- clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
@@ -22,11 +22,19 @@
 
   bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
 
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+    if (Init->getSourceLocation().isInvalid())
+      InvalidLocsVisited = true;
+    Match("initializer", Init->getSourceLocation());
+    return true;
+  }
+
   bool VisitInitListExpr(InitListExpr *ILE) {
     Match(ILE->isSemanticForm() ? "semantic" : "syntactic", ILE->getBeginLoc());
     return true;
   }
 
+  bool InvalidLocsVisited = false;
 private:
   bool VisitImplicitCode;
 };
@@ -49,4 +57,22 @@
                               InitListExprPreOrderVisitor::Lang_C));
 }
 
+TEST(RecursiveASTVisitor, CXXCtorInitializerVisitNoImplicit) {
+  std::vector<bool> Config{true, false};
+  for (bool VisitImplCode : Config) {
+    InitListExprPreOrderVisitor Visitor(VisitImplCode);
+    Visitor.ExpectMatch("initializer", 7, 17);
+    EXPECT_TRUE(Visitor.runOver(R"cpp(
+        class A {};
+        class B : public A {
+          B() {};
+        };
+        class C : public A {
+          C() : A() {}
+        };
+      )cpp",
+                                InitListExprPreOrderVisitor::Lang_CXX));
+    EXPECT_EQ(Visitor.InvalidLocsVisited, VisitImplCode);
+  }
+}
 } // end anonymous namespace
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2023,7 +2023,8 @@
   if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
     // Constructor initializers.
     for (auto *I : Ctor->inits()) {
-      TRY_TO(TraverseConstructorInitializer(I));
+      if (I->isWritten() || getDerived().shouldVisitImplicitCode())
+        TRY_TO(TraverseConstructorInitializer(I));
     }
   }
 

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