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

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65938

Files:
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/unittests/Tooling/CMakeLists.txt
===================================================================
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -28,6 +28,7 @@
   RecursiveASTVisitorTests/ConstructExpr.cpp
   RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
   RecursiveASTVisitorTests/CXXMemberCall.cpp
+  RecursiveASTVisitorTests/CXXMethodDecl.cpp
   RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
   RecursiveASTVisitorTests/DeclRefExpr.cpp
   RecursiveASTVisitorTests/ImplicitCtor.cpp
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2027,7 +2027,13 @@
     }
   }
 
-  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 CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))
+    VisitBody &= !MD->isDefaulted() || getDerived().shouldVisitImplicitCode();
+
+  if (VisitBody) {
     TRY_TO(TraverseStmt(D->getBody())); // Function body.
   }
   return true;
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -54,7 +54,7 @@
   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
@@ -255,6 +255,15 @@
       struct $Class[[Tmpl]] {$TemplateParameter[[T]] $Field[[x]] = 0;};
       extern template struct $Class[[Tmpl]]<float>;
       template struct $Class[[Tmpl]]<double>;
+    )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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to