hokein created this revision. hokein added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov, mgorny. Herald added a project: clang.
This patch adds a simple mechanism to disallow global rename on std symbols. We might extend it to other symbols, e.g. protobuf. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D73450 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/refactor/RenameBlacklist.cpp clang-tools-extra/clangd/refactor/RenameBlacklist.h 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. @@ -450,7 +450,7 @@ for (const auto &RenamePos : Code.points()) { auto RenameResult = rename({RenamePos, NewName, AST, testPath(TU.Filename)}); - ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError(); + ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T; ASSERT_EQ(1u, RenameResult->size()); EXPECT_EQ(applyEdits(std::move(*RenameResult)).front().second, expectedResult(Code, NewName)); @@ -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/RenameBlacklist.h =================================================================== --- /dev/null +++ clang-tools-extra/clangd/refactor/RenameBlacklist.h @@ -0,0 +1,24 @@ +//===--- RenameBlacklist.h - ------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H + +#include "clang/AST/Decl.h" +#include "llvm/ADT/DenseSet.h" + +namespace clang { +namespace clangd { + +// Returns true if the given decl is disallowed for rename. +bool isRenameBlacklisted(const NamedDecl &RenameDecl); + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H Index: clang-tools-extra/clangd/refactor/RenameBlacklist.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/refactor/RenameBlacklist.cpp @@ -0,0 +1,24 @@ +//===--- RenameBlacklist.cpp - ----------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "refactor/RenameBlacklist.h" + +namespace clang { +namespace clangd { + +bool isRenameBlacklisted(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()); +} + +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -14,6 +14,7 @@ #include "Selection.h" #include "SourceCode.h" #include "index/SymbolCollector.h" +#include "refactor/RenameBlacklist.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/Basic/SourceLocation.h" @@ -105,7 +106,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 +121,9 @@ if (RenameDecl.getParentFunctionOrMethod()) return None; + if (isRenameBlacklisted(RenameDecl)) + return ReasonToReject::UnsupportedSymbol; + // Check whether the symbol being rename is indexable. auto &ASTCtx = RenameDecl.getASTContext(); bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts()); @@ -131,11 +135,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; @@ -467,13 +469,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); Index: clang-tools-extra/clangd/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/CMakeLists.txt +++ clang-tools-extra/clangd/CMakeLists.txt @@ -107,6 +107,7 @@ index/dex/Trigram.cpp refactor/Rename.cpp + refactor/RenameBlacklist.cpp refactor/Tweak.cpp LINK_LIBS
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits