Author: Kirill Bobyrev Date: 2022-01-18T09:43:53+01:00 New Revision: 2d9198cec994b91fc13ef6cdd6983c61aaa1f726
URL: https://github.com/llvm/llvm-project/commit/2d9198cec994b91fc13ef6cdd6983c61aaa1f726 DIFF: https://github.com/llvm/llvm-project/commit/2d9198cec994b91fc13ef6cdd6983c61aaa1f726.diff LOG: [clangd] Remove redundant check for renamed symbol origin This is a follow-up on D116643. `isInSystemHeader` check already detects symbols coming from the Standard Library, so searching for the qualified name in StdSymbolMap.inc is no longer necessary. The tests filtering out purely based on the symbol qualified names are removed. Reviewed By: hokein Differential Revision: https://reviews.llvm.org/D117491 Added: Modified: clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index a00d2f51b56c0..b106664f0a446 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -164,18 +164,8 @@ llvm::DenseSet<const NamedDecl *> locateDeclAt(ParsedAST &AST, // to be good candidates for modification. bool isExcluded(const NamedDecl &RenameDecl) { const auto &SM = RenameDecl.getASTContext().getSourceManager(); - if (SM.isInSystemHeader(RenameDecl.getLocation())) - return true; - if (isProtoFile(RenameDecl.getLocation(), SM)) - return true; - // FIXME: Remove this std symbol list, as they should be covered by the - // above isInSystemHeader check. - static const auto *StdSymbols = new llvm::DenseSet<llvm::StringRef>({ -#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name}, -#include "StdSymbolMap.inc" -#undef SYMBOL - }); - return StdSymbols->count(printQualifiedName(RenameDecl)); + return SM.isInSystemHeader(RenameDecl.getLocation()) || + isProtoFile(RenameDecl.getLocation(), SM); } enum class ReasonToReject { diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index e3de3e7411c78..70b0bbc8f073a 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -880,21 +880,6 @@ TEST(RenameTest, Renameable) { void f(X x) {x+^+;})cpp", "no symbol", HeaderFile}, - {R"cpp(// disallow rename on excluded symbols (e.g. std symbols) - namespace std { - class str^ing {}; - } - )cpp", - "not a supported kind", !HeaderFile}, - {R"cpp(// disallow rename on excluded symbols (e.g. std symbols) - namespace std { - inline namespace __u { - class str^ing {}; - } - } - )cpp", - "not a supported kind", !HeaderFile}, - {R"cpp(// disallow rename on non-normal identifiers. @interface Foo {} -(int) fo^o:(int)x; // Token is an identifier, but declaration name isn't a simple identifier. @@ -1199,11 +1184,14 @@ TEST(RenameTest, MainFileReferencesOnly) { } TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) { - // filter out references not from main file. llvm::StringRef Test = R"cpp( + #include <cstdlib> #include <system> + SystemSym^bol abc; + + void foo() { at^oi("9000"); } )cpp"; Annotations Code(Test); @@ -1211,14 +1199,22 @@ TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) { TU.AdditionalFiles["system"] = R"cpp( class SystemSymbol {}; )cpp"; + TU.AdditionalFiles["cstdlib"] = R"cpp( + int atoi(const char *str); + )cpp"; TU.ExtraArgs = {"-isystem", testRoot()}; auto AST = TU.build(); llvm::StringRef NewName = "abcde"; - auto Results = rename({Code.point(), NewName, AST, testPath(TU.Filename)}); - EXPECT_FALSE(Results) << "expected rename returned an error: " << Code.code(); - auto ActualMessage = llvm::toString(Results.takeError()); - EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind")); + // Clangd will not allow renaming symbols from the system headers for + // correctness. + for (auto &Point : Code.points()) { + auto Results = rename({Point, NewName, AST, testPath(TU.Filename)}); + EXPECT_FALSE(Results) << "expected rename returned an error: " + << Code.code(); + auto ActualMessage = llvm::toString(Results.takeError()); + EXPECT_THAT(ActualMessage, testing::HasSubstr("not a supported kind")); + } } TEST(RenameTest, ProtobufSymbolIsExcluded) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
