hokein updated this revision to Diff 240528.
hokein marked 5 inline comments as done.
hokein added a comment.
address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73450/new/
https://reviews.llvm.org/D73450
Files:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -383,12 +383,12 @@
// Typedef.
R"cpp(
- namespace std {
+ namespace ns {
class basic_string {};
typedef basic_string [[s^tring]];
- } // namespace std
+ } // namespace ns
- std::[[s^tring]] foo();
+ ns::[[s^tring]] foo();
)cpp",
// Variable.
@@ -539,6 +539,13 @@
void fo^o() {})cpp",
"used outside main file", !HeaderFile, nullptr /*no index*/},
+ {R"cpp(// disallow rename on blacklisted symbols (e.g. std symbols)
+ namespace std {
+ class str^ing {};
+ }
+ )cpp",
+ "not a supported kind", !HeaderFile, Index},
+
{R"cpp(
void foo(int);
void foo(char);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -96,6 +96,15 @@
return Result;
}
+bool isBlacklisted(const NamedDecl &RenameDecl) {
+ static const auto *StdSymbols = new llvm::DenseSet<llvm::StringRef>({
+#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
+#include "StdSymbolMap.inc"
+#undef SYMBOL
+ });
+ return StdSymbols->count(RenameDecl.getQualifiedNameAsString());
+}
+
enum ReasonToReject {
NoSymbolFound,
NoIndexProvided,
@@ -105,7 +114,7 @@
AmbiguousSymbol,
};
-llvm::Optional<ReasonToReject> renameable(const Decl &RenameDecl,
+llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
StringRef MainFilePath,
const SymbolIndex *Index,
bool CrossFile) {
@@ -120,6 +129,9 @@
if (RenameDecl.getParentFunctionOrMethod())
return None;
+ if (isBlacklisted(RenameDecl))
+ return ReasonToReject::UnsupportedSymbol;
+
// Check whether the symbol being rename is indexable.
auto &ASTCtx = RenameDecl.getASTContext();
bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
@@ -131,11 +143,9 @@
IsMainFileOnly = false;
else if (!DeclaredInMainFile)
IsMainFileOnly = false;
- bool IsIndexable =
- isa<NamedDecl>(RenameDecl) &&
- SymbolCollector::shouldCollectSymbol(
- cast<NamedDecl>(RenameDecl), RenameDecl.getASTContext(),
- SymbolCollector::Options(), IsMainFileOnly);
+ bool IsIndexable = SymbolCollector::shouldCollectSymbol(
+ RenameDecl, RenameDecl.getASTContext(), SymbolCollector::Options(),
+ IsMainFileOnly);
if (!IsIndexable) // If the symbol is not indexable, we disallow rename.
return ReasonToReject::NonIndexable;
@@ -222,7 +232,6 @@
llvm::DenseSet<SymbolID> TargetIDs;
for (auto &USR : RenameUSRs)
TargetIDs.insert(SymbolID(USR));
-
std::vector<SourceLocation> Results;
for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
findExplicitReferences(TopLevelDecl, [&](ReferenceLoc Ref) {
@@ -467,13 +476,13 @@
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
- const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(*DeclsUnderCursor.begin());
+ const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(
+ (*DeclsUnderCursor.begin())->getCanonicalDecl());
if (!RenameDecl)
return makeError(ReasonToReject::UnsupportedSymbol);
- auto Reject =
- renameable(*RenameDecl->getCanonicalDecl(), RInputs.MainFilePath,
- RInputs.Index, RInputs.AllowCrossFile);
+ auto Reject = renameable(*RenameDecl, RInputs.MainFilePath, RInputs.Index,
+ RInputs.AllowCrossFile);
if (Reject)
return makeError(*Reject);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
