Revert "[clangd] Implement rename by using SelectionTree and findExplicitReferences."

This reverts commit 4f80fc2491.

Caused buildbot failure at
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/58251
This commit is contained in:
Wolfgang Pieb 2019-11-18 12:28:04 -08:00
parent 327904d3cf
commit f805c60a09
2 changed files with 46 additions and 462 deletions

View File

@ -8,16 +8,14 @@
#include "refactor/Rename.h"
#include "AST.h"
#include "FindTarget.h"
#include "Logger.h"
#include "ParsedAST.h"
#include "Selection.h"
#include "SourceCode.h"
#include "index/SymbolCollector.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
namespace clang {
namespace clangd {
@ -36,17 +34,6 @@ llvm::Optional<std::string> filePath(const SymbolLocation &Loc,
return *Path;
}
// Returns true if the given location is expanded from any macro body.
bool isInMacroBody(const SourceManager &SM, SourceLocation Loc) {
while (Loc.isMacroID()) {
if (SM.isMacroBodyExpansion(Loc))
return true;
Loc = SM.getImmediateMacroCallerLoc(Loc);
}
return false;
}
// Query the index to find some other files where the Decl is referenced.
llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
const SymbolIndex &Index) {
@ -69,41 +56,12 @@ llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
return OtherFile;
}
llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST,
SourceLocation TokenStartLoc) {
unsigned Offset =
AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
if (!SelectedNode)
return {};
// If the location points to a Decl, we check it is actually on the name
// range of the Decl. This would avoid allowing rename on unrelated tokens.
// ^class Foo {} // SelectionTree returns CXXRecordDecl,
// // we don't attempt to trigger rename on this position.
// FIXME: make this work on destructors, e.g. "~F^oo()".
if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
if (D->getLocation() != TokenStartLoc)
return {};
}
llvm::DenseSet<const Decl *> Result;
for (const auto *D :
targetDecl(SelectedNode->ASTNode,
DeclRelation::Alias | DeclRelation::TemplatePattern))
Result.insert(D);
return Result;
}
enum ReasonToReject {
NoSymbolFound,
NoIndexProvided,
NonIndexable,
UsedOutsideFile,
UnsupportedSymbol,
AmbiguousSymbol,
};
// Check the symbol Decl is renameable (per the index) within the file.
@ -167,8 +125,6 @@ llvm::Error makeError(ReasonToReject Reason) {
return "symbol may be used in other files (not eligible for indexing)";
case UnsupportedSymbol:
return "symbol is not a supported kind (e.g. namespace, macro)";
case AmbiguousSymbol:
return "there are multiple symbols at the given location";
}
llvm_unreachable("unhandled reason kind");
};
@ -178,38 +134,22 @@ llvm::Error makeError(ReasonToReject Reason) {
}
// Return all rename occurrences in the main file.
std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
const NamedDecl &ND) {
// In theory, locateDeclAt should return the primary template. However, if the
// cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
// will be the CXXRecordDecl, for this case, we need to get the primary
// template maunally.
const auto &RenameDecl =
ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
// getUSRsForDeclaration will find other related symbols, e.g. virtual and its
// overriddens, primary template and all explicit specializations.
// FIXME: get rid of the remaining tooling APIs.
std::vector<std::string> RenameUSRs = tooling::getUSRsForDeclaration(
tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext());
llvm::DenseSet<SymbolID> TargetIDs;
for (auto &USR : RenameUSRs)
TargetIDs.insert(SymbolID(USR));
std::vector<SourceLocation> Results;
tooling::SymbolOccurrences
findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
const NamedDecl *CanonicalRenameDecl =
tooling::getCanonicalSymbolDeclaration(RenameDecl);
assert(CanonicalRenameDecl && "RenameDecl must be not null");
std::vector<std::string> RenameUSRs =
tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext());
std::string OldName = CanonicalRenameDecl->getNameAsString();
tooling::SymbolOccurrences Result;
for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
findExplicitReferences(TopLevelDecl, [&](ReferenceLoc Ref) {
if (Ref.Targets.empty())
return;
for (const auto *Target : Ref.Targets) {
auto ID = getSymbolID(Target);
if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
return;
}
Results.push_back(Ref.NameLoc);
});
tooling::SymbolOccurrences RenameInDecl =
tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl);
Result.insert(Result.end(), std::make_move_iterator(RenameInDecl.begin()),
std::make_move_iterator(RenameInDecl.end()));
}
return Results;
return Result;
}
} // namespace
@ -225,41 +165,30 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
return makeError(UnsupportedSymbol);
auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg);
if (DeclsUnderCursor.empty())
return makeError(NoSymbolFound);
if (DeclsUnderCursor.size() > 1)
return makeError(AmbiguousSymbol);
const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(*DeclsUnderCursor.begin());
const auto *RenameDecl =
tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg);
if (!RenameDecl)
return makeError(UnsupportedSymbol);
return makeError(NoSymbolFound);
if (auto Reject =
renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
return makeError(*Reject);
// Rename sometimes returns duplicate edits (which is a bug). A side-effect of
// adding them to a single Replacements object is these are deduplicated.
tooling::Replacements FilteredChanges;
for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
SourceLocation RenameLoc = Loc;
// We don't rename in any macro bodies, but we allow rename the symbol
// spelled in a top-level macro argument in the main file.
if (RenameLoc.isMacroID()) {
if (isInMacroBody(SM, RenameLoc))
continue;
RenameLoc = SM.getSpellingLoc(Loc);
}
// Filter out locations not from main file.
// We traverse only main file decls, but locations could come from an
// non-preamble #include file e.g.
// void test() {
// int f^oo;
// #include "use_foo.inc"
// }
if (!isInsideMainFile(RenameLoc, SM))
for (const tooling::SymbolOccurrence &Rename :
findOccurrencesWithinFile(AST, RenameDecl)) {
// Currently, we only support normal rename (one range) for C/C++.
// FIXME: support multiple-range rename for objective-c methods.
if (Rename.getNameRanges().size() > 1)
continue;
// We shouldn't have conflicting replacements. If there are conflicts, it
// means that we have bugs either in clangd or in Rename library, therefore
// we refuse to perform the rename.
if (auto Err = FilteredChanges.add(tooling::Replacement(
SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
AST.getASTContext().getSourceManager(),
CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName)))
return std::move(Err);
}
return FilteredChanges;

View File

@ -38,27 +38,27 @@ std::string expectedResult(Annotations Test, llvm::StringRef NewName) {
return Result;
}
TEST(RenameTest, WithinFileRename) {
// rename is runnning on all "^" points, and "[[]]" ranges point to the
TEST(RenameTest, SingleFile) {
// "^" points to the position of the rename, and "[[]]" ranges point to the
// identifier that is being renamed.
llvm::StringRef Tests[] = {
// Function.
// Rename function.
R"cpp(
void [[foo^]]() {
void [[foo]]() {
[[fo^o]]();
}
)cpp",
// Type.
// Rename type.
R"cpp(
struct [[foo^]] {};
struct [[foo]]{};
[[foo]] test() {
[[f^oo]] x;
return x;
}
)cpp",
// Local variable.
// Rename variable.
R"cpp(
void bar() {
if (auto [[^foo]] = 5) {
@ -66,313 +66,18 @@ TEST(RenameTest, WithinFileRename) {
}
}
)cpp",
// Rename class, including constructor/destructor.
R"cpp(
class [[F^oo]] {
[[F^oo]]();
~[[Foo]]();
void foo(int x);
};
[[Foo]]::[[Fo^o]]() {}
void [[Foo]]::foo(int x) {}
)cpp",
// Class in template argument.
R"cpp(
class [[F^oo]] {};
template <typename T> void func();
template <typename T> class Baz {};
int main() {
func<[[F^oo]]>();
Baz<[[F^oo]]> obj;
return 0;
}
)cpp",
// Forward class declaration without definition.
R"cpp(
class [[F^oo]];
[[Foo]] *f();
)cpp",
// Class methods overrides.
R"cpp(
struct A {
virtual void [[f^oo]]() {}
};
struct B : A {
void [[f^oo]]() override {}
};
struct C : B {
void [[f^oo]]() override {}
};
void func() {
A().[[f^oo]]();
B().[[f^oo]]();
C().[[f^oo]]();
}
)cpp",
// Template class (partial) specializations.
R"cpp(
template <typename T>
class [[F^oo]] {};
template<>
class [[F^oo]]<bool> {};
template <typename T>
class [[F^oo]]<T*> {};
void test() {
[[Foo]]<int> x;
[[Foo]]<bool> y;
[[Foo]]<int*> z;
}
)cpp",
// Template class instantiations.
R"cpp(
template <typename T>
class [[F^oo]] {
public:
T foo(T arg, T& ref, T* ptr) {
T value;
int number = 42;
value = (T)number;
value = static_cast<T>(number);
return value;
}
static void foo(T value) {}
T member;
};
template <typename T>
void func() {
[[F^oo]]<T> obj;
obj.member = T();
[[Foo]]<T>::foo();
}
void test() {
[[F^oo]]<int> i;
i.member = 0;
[[F^oo]]<int>::foo(0);
[[F^oo]]<bool> b;
b.member = false;
[[Foo]]<bool>::foo(false);
}
)cpp",
// Template class methods.
R"cpp(
template <typename T>
class A {
public:
void [[f^oo]]() {}
};
void func() {
A<int>().[[f^oo]]();
A<double>().[[f^oo]]();
A<float>().[[f^oo]]();
}
)cpp",
// Complicated class type.
R"cpp(
// Forward declaration.
class [[Fo^o]];
class Baz {
virtual int getValue() const = 0;
};
class [[F^oo]] : public Baz {
public:
[[Foo]](int value = 0) : x(value) {}
[[Foo]] &operator++(int);
bool operator<([[Foo]] const &rhs);
int getValue() const;
private:
int x;
};
void func() {
[[Foo]] *Pointer = 0;
[[Foo]] Variable = [[Foo]](10);
for ([[Foo]] it; it < Variable; it++);
const [[Foo]] *C = new [[Foo]]();
const_cast<[[Foo]] *>(C)->getValue();
[[Foo]] foo;
const Baz &BazReference = foo;
const Baz *BazPointer = &foo;
dynamic_cast<const [[^Foo]] &>(BazReference).getValue();
dynamic_cast<const [[^Foo]] *>(BazPointer)->getValue();
reinterpret_cast<const [[^Foo]] *>(BazPointer)->getValue();
static_cast<const [[^Foo]] &>(BazReference).getValue();
static_cast<const [[^Foo]] *>(BazPointer)->getValue();
}
)cpp",
// CXXConstructor initializer list.
R"cpp(
class Baz {};
class Qux {
Baz [[F^oo]];
public:
Qux();
};
Qux::Qux() : [[F^oo]]() {}
)cpp",
// DeclRefExpr.
R"cpp(
class C {
public:
static int [[F^oo]];
};
int foo(int x);
#define MACRO(a) foo(a)
void func() {
C::[[F^oo]] = 1;
MACRO(C::[[Foo]]);
int y = C::[[F^oo]];
}
)cpp",
// Macros.
R"cpp(
// no rename inside macro body.
#define M1 foo
#define M2(x) x
int [[fo^o]]();
void boo(int);
void qoo() {
[[foo]]();
boo([[foo]]());
M1();
boo(M1());
M2([[foo]]());
M2(M1()); // foo is inside the nested macro body.
}
)cpp",
// MemberExpr in macros
R"cpp(
class Baz {
public:
int [[F^oo]];
};
int qux(int x);
#define MACRO(a) qux(a)
int main() {
Baz baz;
baz.[[Foo]] = 1;
MACRO(baz.[[Foo]]);
int y = baz.[[Foo]];
}
)cpp",
// Template parameters.
R"cpp(
template <typename [[^T]]>
class Foo {
[[T]] foo([[T]] arg, [[T]]& ref, [[^T]]* ptr) {
[[T]] value;
int number = 42;
value = ([[T]])number;
value = static_cast<[[^T]]>(number);
return value;
}
static void foo([[T]] value) {}
[[T]] member;
};
)cpp",
// Typedef.
R"cpp(
namespace std {
class basic_string {};
typedef basic_string [[s^tring]];
} // namespace std
std::[[s^tring]] foo();
)cpp",
// Variable.
R"cpp(
namespace A {
int [[F^oo]];
}
int Foo;
int Qux = Foo;
int Baz = A::[[^Foo]];
void fun() {
struct {
int Foo;
} b = {100};
int Foo = 100;
Baz = Foo;
{
extern int Foo;
Baz = Foo;
Foo = A::[[F^oo]] + Baz;
A::[[Fo^o]] = b.Foo;
}
Foo = b.Foo;
}
)cpp",
// Namespace alias.
R"cpp(
namespace a { namespace b { void foo(); } }
namespace [[^x]] = a::b;
void bar() {
[[x]]::foo();
}
)cpp",
// Scope enums.
R"cpp(
enum class [[K^ind]] { ABC };
void ff() {
[[K^ind]] s;
s = [[Kind]]::ABC;
}
)cpp",
// template class in template argument list.
R"cpp(
template<typename T>
class [[Fo^o]] {};
template <template<typename> class Z> struct Bar { };
template <> struct Bar<[[Foo]]> {};
)cpp",
};
for (const auto T : Tests) {
Annotations Code(T);
auto TU = TestTU::withCode(Code.code());
auto AST = TU.build();
EXPECT_TRUE(AST.getDiagnostics().empty())
<< AST.getDiagnostics().front() << Code.code();
llvm::StringRef NewName = "abcde";
for (const auto &RenamePos : Code.points()) {
auto RenameResult =
renameWithinFile(AST, testPath(TU.Filename), RenamePos, NewName);
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T;
auto ApplyResult = llvm::cantFail(
tooling::applyAllReplacements(Code.code(), *RenameResult));
EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
}
auto RenameResult =
renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
auto ApplyResult = llvm::cantFail(
tooling::applyAllReplacements(Code.code(), *RenameResult));
EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
}
}
@ -443,32 +148,12 @@ TEST(RenameTest, Renameable) {
{R"cpp(// foo is declared outside the file.
void fo^o() {}
)cpp",
"used outside main file", !HeaderFile /*cc file*/, Index},
)cpp", "used outside main file", !HeaderFile /*cc file*/, Index},
{R"cpp(
// We should detect the symbol is used outside the file from the AST.
void fo^o() {})cpp",
"used outside main file", !HeaderFile, nullptr /*no index*/},
{R"cpp(
void foo(int);
void foo(char);
template <typename T> void f(T t) {
fo^o(t);
})cpp",
"multiple symbols", !HeaderFile, nullptr /*no index*/},
{R"cpp(// disallow rename on unrelated token.
cl^ass Foo {};
)cpp",
"no symbol", !HeaderFile, nullptr},
{R"cpp(// disallow rename on unrelated token.
temp^late<typename T>
class Foo {};
)cpp",
"no symbol", !HeaderFile, nullptr},
};
for (const auto& Case : Cases) {
@ -506,36 +191,6 @@ TEST(RenameTest, Renameable) {
}
}
TEST(RenameTest, MainFileReferencesOnly) {
// filter out references not from main file.
llvm::StringRef Test =
R"cpp(
void test() {
int [[fo^o]] = 1;
// rename references not from main file are not included.
#include "foo.inc"
})cpp";
Annotations Code(Test);
auto TU = TestTU::withCode(Code.code());
TU.AdditionalFiles["foo.inc"] = R"cpp(
#define Macro(X) X
&Macro(foo);
&foo;
)cpp";
auto AST = TU.build();
EXPECT_TRUE(AST.getDiagnostics().empty())
<< AST.getDiagnostics().front() << Code.code();
llvm::StringRef NewName = "abcde";
auto RenameResult =
renameWithinFile(AST, testPath(TU.Filename), Code.point(), NewName);
ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << Code.point();
auto ApplyResult =
llvm::cantFail(tooling::applyAllReplacements(Code.code(), *RenameResult));
EXPECT_EQ(expectedResult(Code, NewName), ApplyResult);
}
} // namespace
} // namespace clangd
} // namespace clang