[AST] No longer visiting CXXMethodDecl bodies created by compiler when method was...

Summary:
Clang generates function bodies and puts them in the AST for default methods if it is defaulted outside the class definition.

`
struct A {
   A &operator=(A &&O);
};

A &A::operator=(A &&O) = default;
`

This will generate a function body for the `A &A::operator=(A &&O)` and put it in the AST. This body should not be visited if implicit code is not visited as it is implicit.

This was causing SemanticHighlighting in clangd to generate duplicate tokens and putting them in weird places.

Reviewers: hokein, ilya-biryukov, gribozavr

Subscribers: mgorny, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D65938

llvm-svn: 368402

diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 3ff7bbc8bbb30dbbedd7a84a89da21b320b05c16..4405134461dde5f76f30fec20015d877d5bd710e 100644 (file)
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -55,7 +55,7 @@ void checkHighlightings(llvm::StringRef Code) {
   Annotations Test(Code);
   auto AST = TestTU::withCode(Test.code()).build();
   std::vector<HighlightingToken> ActualTokens = getSemanticHighlightings(AST);
-  EXPECT_THAT(ActualTokens, getExpectedTokens(Test));
+  EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
 
 // Any annotations in OldCode and NewCode are converted into their corresponding
@@ -276,6 +276,15 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
         $Class[[Foo]] *$Variable[[FP]] = ($Class[[Foo]]*)$Variable[[B]];
         $Primitive[[int]] $Variable[[I]] = ($Primitive[[int]])$Variable[[B]];
       }
+    )cpp"
+    R"cpp(
+      struct $Class[[B]] {};
+      struct $Class[[A]] {
+        $Class[[B]] $Field[[BB]];
+        $Class[[A]] &operator=($Class[[A]] &&$Variable[[O]]);
+      };
+
+      $Class[[A]] &$Class[[A]]::operator=($Class[[A]] &&$Variable[[O]]) = default;
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);

diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 918e5b084d928cac7cca7412547382558f0f5ee9..924cf832a8a7c7b81e2b7ab6362ba1090b41938b 100644 (file)
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2027,7 +2027,13 @@ bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
     }
   }
 
-  if (D->isThisDeclarationADefinition()) {
+  bool VisitBody = D->isThisDeclarationADefinition();
+  // If a method is set to default outside the class definition the compiler
+  // generates the method body and adds it to the AST.
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(D))
+    VisitBody &= !MD->isDefaulted() || getDerived().shouldVisitImplicitCode();
+
+  if (VisitBody) {
     TRY_TO(TraverseStmt(D->getBody())); // Function body.
   }
   return true;

diff --git a/clang/unittests/Tooling/CMakeLists.txt b/clang/unittests/Tooling/CMakeLists.txt
index 99a3e0f209f1021cbdad1131c5250745fb504447..c699d41c2846e82cc8f881bc71b169a0f1f5081f 100644 (file)
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -28,6 +28,7 @@ add_clang_unittest(ToolingTests
   RecursiveASTVisitorTests/ConstructExpr.cpp
   RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
   RecursiveASTVisitorTests/CXXMemberCall.cpp
+  RecursiveASTVisitorTests/CXXMethodDecl.cpp
   RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
   RecursiveASTVisitorTests/DeclRefExpr.cpp
   RecursiveASTVisitorTests/ImplicitCtor.cpp

diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
new file mode 100644 (file)
index 0000000..6441ea9
--- /dev/null
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
@@ -0,0 +1,58 @@
+//=------ unittest/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp ------=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestVisitor.h"
+#include "clang/AST/Expr.h"
+
+using namespace clang;
+
+namespace {
+
+class CXXMethodDeclVisitor
+    : public ExpectedLocationVisitor<CXXMethodDeclVisitor> {
+public:
+  CXXMethodDeclVisitor(bool VisitImplicitCode)
+      : VisitImplicitCode(VisitImplicitCode) {}
+
+  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *D) {
+    Match("declref", D->getLocation());
+    return true;
+  }
+  bool VisitParmVarDecl(ParmVarDecl *P) {
+    Match("parm", P->getLocation());
+    return true;
+  }
+
+private:
+  bool VisitImplicitCode;
+};
+
+TEST(RecursiveASTVisitor, CXXMethodDeclNoDefaultBodyVisited) {
+  for (bool VisitImplCode : {false, true}) {
+    CXXMethodDeclVisitor Visitor(VisitImplCode);
+    if (VisitImplCode)
+      Visitor.ExpectMatch("declref", 8, 28);
+    else
+      Visitor.DisallowMatch("declref", 8, 28);
+
+    Visitor.ExpectMatch("parm", 8, 27);
+    llvm::StringRef Code = R"cpp(
+      struct B {};
+      struct A {
+        B BB;
+        A &operator=(A &&O);
+      };
+
+      A &A::operator=(A &&O) = default;
+    )cpp";
+    EXPECT_TRUE(Visitor.runOver(Code, CXXMethodDeclVisitor::Lang_CXX11));
+  }
+}
+} // end anonymous namespace