[clang-tools-extra] 2d9198c - [clangd] Remove redundant check for renamed symbol origin

[Kirill Bobyrev via cfe-commits]

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) {