From 902dc6c69ce7985427efa103a7c4099c372da6fa Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Fri, 29 Nov 2019 14:58:44 +0100 Subject: [PATCH] [clangd] Fix a regression issue in local rename. Summary: The regression is that we can't rename symbols in annonymous namespaces. Reviewers: ilya-biryukov Subscribers: MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D70853 --- clang-tools-extra/clangd/refactor/Rename.cpp | 18 ++++++++++++------ .../clangd/unittests/RenameTests.cpp | 9 ++++++++- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 6a3439cc0612..7d74641be719 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -123,20 +123,26 @@ llvm::Optional renameable(const Decl &RenameDecl, if (RenameDecl.getParentFunctionOrMethod()) return None; + // Check whether the symbol being rename is indexable. + auto &ASTCtx = RenameDecl.getASTContext(); + bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts()); + bool DeclaredInMainFile = + isInsideMainFile(RenameDecl.getBeginLoc(), ASTCtx.getSourceManager()); + bool IsMainFileOnly = true; + if (MainFileIsHeader) + // main file is a header, the symbol can't be main file only. + IsMainFileOnly = false; + else if (!DeclaredInMainFile) + IsMainFileOnly = false; bool IsIndexable = isa(RenameDecl) && SymbolCollector::shouldCollectSymbol( cast(RenameDecl), RenameDecl.getASTContext(), - SymbolCollector::Options(), CrossFile); + SymbolCollector::Options(), IsMainFileOnly); if (!IsIndexable) // If the symbol is not indexable, we disallow rename. return ReasonToReject::NonIndexable; if (!CrossFile) { - auto &ASTCtx = RenameDecl.getASTContext(); - const auto &SM = ASTCtx.getSourceManager(); - bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts()); - bool DeclaredInMainFile = isInsideMainFile(RenameDecl.getBeginLoc(), SM); - if (!DeclaredInMainFile) // We are sure the symbol is used externally, bail out early. return ReasonToReject::UsedOutsideFile; diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index 0615272de372..1ade0c0443bc 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -450,13 +450,20 @@ TEST(RenameTest, Renameable) { )cpp", "used outside main file", HeaderFile, Index}, - {R"cpp(// disallow -- symbol is not indexable. + {R"cpp(// disallow -- symbol in annonymous namespace in header is not indexable. namespace { class Unin^dexable {}; } )cpp", "not eligible for indexing", HeaderFile, Index}, + {R"cpp(// allow -- symbol in annonymous namespace in non-header file is indexable. + namespace { + class [[F^oo]] {}; + } + )cpp", + nullptr, !HeaderFile, Index}, + {R"cpp(// disallow -- namespace symbol isn't supported namespace n^s {} )cpp",