hokein created this revision. hokein added a reviewer: kbobyrev. Herald added subscribers: usaxena95, kadircet, arphaman. hokein requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.
Fixes https://github.com/clangd/clangd/issues/963. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D116643 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 @@ -1198,6 +1198,29 @@ expectedResult(Code, NewName)); } +TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) { + // filter out references not from main file. + llvm::StringRef Test = + R"cpp( + #include <system> + SystemSym^bol abc; + )cpp"; + + Annotations Code(Test); + auto TU = TestTU::withCode(Code.code()); + TU.AdditionalFiles["system"] = R"cpp( + class SystemSymbol {}; + )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")); +} + TEST(RenameTest, ProtobufSymbolIsExcluded) { Annotations Code("Prot^obuf buf;"); auto TU = TestTU::withCode(Code.code()); Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -159,13 +159,17 @@ return Result; } -// By default, we exclude C++ standard symbols and protobuf symbols as rename -// these symbols would change system/generated files which are unlikely to be -// modified. +// By default, we exclude symbols from system headers and protobuf symbols as +// rename these symbols would change system/generated files which are unlikely +// to be modified. bool isExcluded(const NamedDecl &RenameDecl) { - if (isProtoFile(RenameDecl.getLocation(), - RenameDecl.getASTContext().getSourceManager())) + 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"
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/RenameTests.cpp +++ clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1198,6 +1198,29 @@ expectedResult(Code, NewName)); } +TEST(RenameTest, NoRenameOnSymbolsFromSystemHeaders) { + // filter out references not from main file. + llvm::StringRef Test = + R"cpp( + #include <system> + SystemSym^bol abc; + )cpp"; + + Annotations Code(Test); + auto TU = TestTU::withCode(Code.code()); + TU.AdditionalFiles["system"] = R"cpp( + class SystemSymbol {}; + )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")); +} + TEST(RenameTest, ProtobufSymbolIsExcluded) { Annotations Code("Prot^obuf buf;"); auto TU = TestTU::withCode(Code.code()); Index: clang-tools-extra/clangd/refactor/Rename.cpp =================================================================== --- clang-tools-extra/clangd/refactor/Rename.cpp +++ clang-tools-extra/clangd/refactor/Rename.cpp @@ -159,13 +159,17 @@ return Result; } -// By default, we exclude C++ standard symbols and protobuf symbols as rename -// these symbols would change system/generated files which are unlikely to be -// modified. +// By default, we exclude symbols from system headers and protobuf symbols as +// rename these symbols would change system/generated files which are unlikely +// to be modified. bool isExcluded(const NamedDecl &RenameDecl) { - if (isProtoFile(RenameDecl.getLocation(), - RenameDecl.getASTContext().getSourceManager())) + 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"
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits