[PATCH] D117491: [clangd] Remove redundant check for renamed symbol origin

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: hokein.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is a follow-up on D116643 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117491

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
@@ -880,21 +880,6 @@
         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, 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 @@
   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) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -164,18 +164,8 @@
 // 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 {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117491.400541.patch
Type: text/x-patch
Size: 3464 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220117/755b9b16/attachment.bin>