[clang-tools-extra] bbcbb10 - [clangd] Ignore more implicit nodes in computing selection.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-11-14T17:35:00+01:00
New Revision: bbcbb10e2d0848b611c51b1ff56c758645c55f3b

URL: https://github.com/llvm/llvm-project/commit/bbcbb10e2d0848b611c51b1ff56c758645c55f3b
DIFF: https://github.com/llvm/llvm-project/commit/bbcbb10e2d0848b611c51b1ff56c758645c55f3b.diff

LOG: [clangd] Ignore more implicit nodes in computing selection.

Summary:
The DeclRefExpr for the callee of overloaded `operator()` and `operator[]` are
assigned the range of the paren/bracket lists in the AST.
These are better thought of as implicit (at least `()` - `[] is murkier).
But there's no bit on Expr for implicit, so just ignore them on our side.

While here, deal with the case where an implicit stmt (e.g. implicit-this)
is wrapped in an implicit cast. Previously we ignored the statement but not
the cast, and so the cast ended up being selected.

Fixes https://github.com/clangd/clangd/issues/195

Reviewers: kadircet, lh123

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/Selection.cpp
    clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 8451c988ab66..1424825d82bc 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -10,9 +10,11 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
@@ -145,6 +147,32 @@ std::string printNodeToString(const DynTypedNode &N, const PrintingPolicy &PP) {
 }
 #endif
 
+bool isImplicit(const Stmt* S) {
+  // Some Stmts are implicit and shouldn't be traversed, but there's no
+  // "implicit" attribute on Stmt/Expr.
+  // Unwrap implicit casts first if present (other nodes too?).
+  if (auto *ICE = llvm::dyn_cast<ImplicitCastExpr>(S))
+    S = ICE->getSubExprAsWritten();
+  // Implicit this in a MemberExpr is not filtered out by RecursiveASTVisitor.
+  // It would be nice if RAV handled this (!shouldTraverseImplicitCode()).
+  if (auto *CTI = llvm::dyn_cast<CXXThisExpr>(S))
+    if (CTI->isImplicit())
+      return true;
+  // Refs to operator() and [] are (almost?) always implicit as part of calls.
+  if (auto *DRE = llvm::dyn_cast<DeclRefExpr>(S)) {
+    if (auto *FD = llvm::dyn_cast<FunctionDecl>(DRE->getDecl())) {
+      switch (FD->getOverloadedOperator()) {
+      case OO_Call:
+      case OO_Subscript:
+        return true;
+      default:
+        break;
+      }
+    }
+  }
+  return false;
+}
+
 // We find the selection by visiting written nodes in the AST, looking for nodes
 // that intersect with the selected character range.
 //
@@ -210,13 +238,8 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
   }
   // Stmt is the same, but this form allows the data recursion optimization.
   bool dataTraverseStmtPre(Stmt *X) {
-    if (!X)
+    if (!X || isImplicit(X))
       return false;
-    // Implicit this in a MemberExpr is not filtered out by RecursiveASTVisitor.
-    // It would be nice if RAV handled this (!shouldTRaverseImplicitCode()).
-    if (auto *CTI = llvm::dyn_cast<CXXThisExpr>(X))
-      if (CTI->isImplicit())
-        return false;
     auto N = DynTypedNode::create(*X);
     if (canSafelySkipNode(N))
       return false;

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 309f4c0dbcd0..8895dd82262d 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -213,11 +213,18 @@ TEST(SelectionTest, CommonAncestor) {
       {
           R"cpp(
             struct S {
-              int foo;
-              int bar() { return [[f^oo]]; }
+              int foo() const;
+              int bar() { return [[f^oo]](); }
             };
           )cpp",
-          "MemberExpr", // Not implicit CXXThisExpr!
+          "MemberExpr", // Not implicit CXXThisExpr, or its implicit cast!
+      },
+      {
+          R"cpp(
+            auto lambda = [](const char*){ return 0; };
+            int x = lambda([["y^"]]);
+          )cpp",
+          "StringLiteral", // Not DeclRefExpr to operator()!
       },
 
       // Point selections.