Skip to content

Commit

Permalink
[clangd] Ignore more implicit nodes in computing selection.
Browse files Browse the repository at this point in the history
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 clangd/clangd#195

Reviewers: kadircet, lh123

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

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70194
  • Loading branch information
sam-mccall committed Nov 14, 2019
1 parent 37abeed commit bbcbb10
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
35 changes: 29 additions & 6 deletions clang-tools-extra/clangd/Selection.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 10 additions & 3 deletions clang-tools-extra/clangd/unittests/SelectionTests.cpp
Expand Up @@ -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.
Expand Down

0 comments on commit bbcbb10

Please sign in to comment.