Skip to content

Commit 7db1230

Browse files
committed
Reland "[clangd] Implement rename by using SelectionTree and findExplicitReferences."
this reland the commit 4f80fc2 which has been reverted at f805c60. Fixed windows buildbot failure (by adding -fno-delayed-template-parsing flag).
1 parent fd03be3 commit 7db1230

File tree

2 files changed

+459
-48
lines changed

2 files changed

+459
-48
lines changed

clang-tools-extra/clangd/refactor/Rename.cpp

Lines changed: 103 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@
88

99
#include "refactor/Rename.h"
1010
#include "AST.h"
11+
#include "FindTarget.h"
1112
#include "Logger.h"
1213
#include "ParsedAST.h"
14+
#include "Selection.h"
1315
#include "SourceCode.h"
1416
#include "index/SymbolCollector.h"
15-
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
16-
#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
17+
#include "clang/AST/DeclCXX.h"
18+
#include "clang/AST/DeclTemplate.h"
19+
#include "clang/Basic/SourceLocation.h"
1720
#include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
18-
#include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
1921

2022
namespace clang {
2123
namespace clangd {
@@ -34,6 +36,17 @@ llvm::Optional<std::string> filePath(const SymbolLocation &Loc,
3436
return *Path;
3537
}
3638

39+
// Returns true if the given location is expanded from any macro body.
40+
bool isInMacroBody(const SourceManager &SM, SourceLocation Loc) {
41+
while (Loc.isMacroID()) {
42+
if (SM.isMacroBodyExpansion(Loc))
43+
return true;
44+
Loc = SM.getImmediateMacroCallerLoc(Loc);
45+
}
46+
47+
return false;
48+
}
49+
3750
// Query the index to find some other files where the Decl is referenced.
3851
llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
3952
const SymbolIndex &Index) {
@@ -56,12 +69,41 @@ llvm::Optional<std::string> getOtherRefFile(const Decl &D, StringRef MainFile,
5669
return OtherFile;
5770
}
5871

72+
llvm::DenseSet<const Decl *> locateDeclAt(ParsedAST &AST,
73+
SourceLocation TokenStartLoc) {
74+
unsigned Offset =
75+
AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
76+
77+
SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
78+
const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
79+
if (!SelectedNode)
80+
return {};
81+
82+
// If the location points to a Decl, we check it is actually on the name
83+
// range of the Decl. This would avoid allowing rename on unrelated tokens.
84+
// ^class Foo {} // SelectionTree returns CXXRecordDecl,
85+
// // we don't attempt to trigger rename on this position.
86+
// FIXME: make this work on destructors, e.g. "~F^oo()".
87+
if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
88+
if (D->getLocation() != TokenStartLoc)
89+
return {};
90+
}
91+
92+
llvm::DenseSet<const Decl *> Result;
93+
for (const auto *D :
94+
targetDecl(SelectedNode->ASTNode,
95+
DeclRelation::Alias | DeclRelation::TemplatePattern))
96+
Result.insert(D);
97+
return Result;
98+
}
99+
59100
enum ReasonToReject {
60101
NoSymbolFound,
61102
NoIndexProvided,
62103
NonIndexable,
63104
UsedOutsideFile,
64105
UnsupportedSymbol,
106+
AmbiguousSymbol,
65107
};
66108

67109
// Check the symbol Decl is renameable (per the index) within the file.
@@ -125,6 +167,8 @@ llvm::Error makeError(ReasonToReject Reason) {
125167
return "symbol may be used in other files (not eligible for indexing)";
126168
case UnsupportedSymbol:
127169
return "symbol is not a supported kind (e.g. namespace, macro)";
170+
case AmbiguousSymbol:
171+
return "there are multiple symbols at the given location";
128172
}
129173
llvm_unreachable("unhandled reason kind");
130174
};
@@ -134,22 +178,38 @@ llvm::Error makeError(ReasonToReject Reason) {
134178
}
135179

136180
// Return all rename occurrences in the main file.
137-
tooling::SymbolOccurrences
138-
findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
139-
const NamedDecl *CanonicalRenameDecl =
140-
tooling::getCanonicalSymbolDeclaration(RenameDecl);
141-
assert(CanonicalRenameDecl && "RenameDecl must be not null");
142-
std::vector<std::string> RenameUSRs =
143-
tooling::getUSRsForDeclaration(CanonicalRenameDecl, AST.getASTContext());
144-
std::string OldName = CanonicalRenameDecl->getNameAsString();
145-
tooling::SymbolOccurrences Result;
181+
std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
182+
const NamedDecl &ND) {
183+
// In theory, locateDeclAt should return the primary template. However, if the
184+
// cursor is under the underlying CXXRecordDecl of the ClassTemplateDecl, ND
185+
// will be the CXXRecordDecl, for this case, we need to get the primary
186+
// template maunally.
187+
const auto &RenameDecl =
188+
ND.getDescribedTemplate() ? *ND.getDescribedTemplate() : ND;
189+
// getUSRsForDeclaration will find other related symbols, e.g. virtual and its
190+
// overriddens, primary template and all explicit specializations.
191+
// FIXME: get rid of the remaining tooling APIs.
192+
std::vector<std::string> RenameUSRs = tooling::getUSRsForDeclaration(
193+
tooling::getCanonicalSymbolDeclaration(&RenameDecl), AST.getASTContext());
194+
llvm::DenseSet<SymbolID> TargetIDs;
195+
for (auto &USR : RenameUSRs)
196+
TargetIDs.insert(SymbolID(USR));
197+
198+
std::vector<SourceLocation> Results;
146199
for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
147-
tooling::SymbolOccurrences RenameInDecl =
148-
tooling::getOccurrencesOfUSRs(RenameUSRs, OldName, TopLevelDecl);
149-
Result.insert(Result.end(), std::make_move_iterator(RenameInDecl.begin()),
150-
std::make_move_iterator(RenameInDecl.end()));
200+
findExplicitReferences(TopLevelDecl, [&](ReferenceLoc Ref) {
201+
if (Ref.Targets.empty())
202+
return;
203+
for (const auto *Target : Ref.Targets) {
204+
auto ID = getSymbolID(Target);
205+
if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
206+
return;
207+
}
208+
Results.push_back(Ref.NameLoc);
209+
});
151210
}
152-
return Result;
211+
212+
return Results;
153213
}
154214

155215
} // namespace
@@ -165,30 +225,41 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
165225
if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))
166226
return makeError(UnsupportedSymbol);
167227

168-
const auto *RenameDecl =
169-
tooling::getNamedDeclAt(AST.getASTContext(), SourceLocationBeg);
170-
if (!RenameDecl)
228+
auto DeclsUnderCursor = locateDeclAt(AST, SourceLocationBeg);
229+
if (DeclsUnderCursor.empty())
171230
return makeError(NoSymbolFound);
231+
if (DeclsUnderCursor.size() > 1)
232+
return makeError(AmbiguousSymbol);
233+
234+
const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(*DeclsUnderCursor.begin());
235+
if (!RenameDecl)
236+
return makeError(UnsupportedSymbol);
172237

173238
if (auto Reject =
174239
renamableWithinFile(*RenameDecl->getCanonicalDecl(), File, Index))
175240
return makeError(*Reject);
176241

177-
// Rename sometimes returns duplicate edits (which is a bug). A side-effect of
178-
// adding them to a single Replacements object is these are deduplicated.
179242
tooling::Replacements FilteredChanges;
180-
for (const tooling::SymbolOccurrence &Rename :
181-
findOccurrencesWithinFile(AST, RenameDecl)) {
182-
// Currently, we only support normal rename (one range) for C/C++.
183-
// FIXME: support multiple-range rename for objective-c methods.
184-
if (Rename.getNameRanges().size() > 1)
243+
for (SourceLocation Loc : findOccurrencesWithinFile(AST, *RenameDecl)) {
244+
SourceLocation RenameLoc = Loc;
245+
// We don't rename in any macro bodies, but we allow rename the symbol
246+
// spelled in a top-level macro argument in the main file.
247+
if (RenameLoc.isMacroID()) {
248+
if (isInMacroBody(SM, RenameLoc))
249+
continue;
250+
RenameLoc = SM.getSpellingLoc(Loc);
251+
}
252+
// Filter out locations not from main file.
253+
// We traverse only main file decls, but locations could come from an
254+
// non-preamble #include file e.g.
255+
// void test() {
256+
// int f^oo;
257+
// #include "use_foo.inc"
258+
// }
259+
if (!isInsideMainFile(RenameLoc, SM))
185260
continue;
186-
// We shouldn't have conflicting replacements. If there are conflicts, it
187-
// means that we have bugs either in clangd or in Rename library, therefore
188-
// we refuse to perform the rename.
189261
if (auto Err = FilteredChanges.add(tooling::Replacement(
190-
AST.getASTContext().getSourceManager(),
191-
CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName)))
262+
SM, CharSourceRange::getTokenRange(RenameLoc), NewName)))
192263
return std::move(Err);
193264
}
194265
return FilteredChanges;

0 commit comments

Comments
 (0)