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

[jvikstrom]

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.

[hokein]

looks good from my side.

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

262 ↗

(On Diff #214109)

nit: the indent should be 2 spaces.

clang/include/clang/AST/RecursiveASTVisitor.h

2033 ↗

(On Diff #214109)

nit: use auto, as type in RHS is obvious.

cfe/trunk/include/clang/AST/RecursiveASTVisitor.h

bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
   if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
     // Constructor initializers.
     for (auto *I : Ctor->inits()) {
       if (I->isWritten() || getDerived().shouldVisitImplicitCode())
         TRY_TO(TraverseConstructorInitializer(I));
     }
   }
 
-  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;
 }
 
 DEF_TRAVERSE_DECL(FunctionDecl, {
   // We skip decls_begin/decls_end, which are already covered by
   // TraverseFunctionHelper().

cfe/trunk/unittests/Tooling/CMakeLists.txt

add_clang_unittest(ToolingTests
   LookupTest.cpp
   QualTypeNamesTest.cpp
   RangeSelectorTest.cpp
   RecursiveASTVisitorTests/Attr.cpp
   RecursiveASTVisitorTests/Class.cpp
   RecursiveASTVisitorTests/ConstructExpr.cpp
   RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
   RecursiveASTVisitorTests/CXXMemberCall.cpp
+  RecursiveASTVisitorTests/CXXMethodDecl.cpp
   RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
   RecursiveASTVisitorTests/DeclRefExpr.cpp
   RecursiveASTVisitorTests/ImplicitCtor.cpp
   RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp
   RecursiveASTVisitorTests/InitListExprPostOrder.cpp
   RecursiveASTVisitorTests/InitListExprPostOrderNoQueue.cpp
   RecursiveASTVisitorTests/InitListExprPreOrder.cpp
   RecursiveASTVisitorTests/InitListExprPreOrderNoQueue.cpp

cfe/trunk/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp

+//=------ 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

clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

std::vector<HighlightingToken> getExpectedTokens(Annotations &Test) {
   llvm::sort(ExpectedTokens);
   return ExpectedTokens;
 }
 
 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
 // HighlightingToken. The tokens are diffed against each other. Any lines where
 // the tokens should diff must be marked with a ^ somewhere on that line in
 // NewCode. If there are diffs that aren't marked with ^ the test fails. The
 // test also fails if there are lines marked with ^ that don't differ.
 void checkDiffedHighlights(llvm::StringRef OldCode, llvm::StringRef NewCode) {
R"cpp(
         operator $Class[[Foo]]();
       };
       $Primitive[[void]] $Function[[f]]() {
         $Class[[Bar]] $Variable[[B]];
         $Class[[Foo]] $Variable[[F]] = $Variable[[B]];
         $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);
   }
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
   class HighlightingsCounterDiagConsumer : public DiagnosticsConsumer {