[PATCH] D70853: [clangd] Fix a regression issue in local rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

The regression is that we can't rename symbols in annonymous
namespaces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70853

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
@@ -450,13 +450,20 @@
       )cpp",
        "used outside main file", HeaderFile, Index},
 
-      {R"cpp(// disallow -- symbol is not indexable.
+      {R"cpp(// disallow -- symbol in annonymous namespace in header is not indexable.
         namespace {
         class Unin^dexable {};
         }
       )cpp",
        "not eligible for indexing", HeaderFile, Index},
 
+      {R"cpp(// allow -- symbol in annonymous namespace in non-header file is indexable.
+        namespace {
+        class [[F^oo]] {};
+        }
+      )cpp",
+       nullptr, !HeaderFile, Index},
+
       {R"cpp(// disallow -- namespace symbol isn't supported
         namespace n^s {}
       )cpp",
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -123,20 +123,27 @@
   if (RenameDecl.getParentFunctionOrMethod())
     return None;
 
+  // Check whether the symbol being rename is indexable.
+  auto &ASTCtx = RenameDecl.getASTContext();
+  bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
+  bool DeclaredInMainFile =
+      isInsideMainFile(RenameDecl.getBeginLoc(), ASTCtx.getSourceManager());
+  bool IsMainFileOnly = true;
+  if (MainFileIsHeader)
+    // main file is a header, the symbol can't be main file only.
+    IsMainFileOnly = false;
+  else if (!DeclaredInMainFile)
+    // the symbol declared outside of the main file, can't be main file only.
+    IsMainFileOnly = false;
   bool IsIndexable =
       isa<NamedDecl>(RenameDecl) &&
       SymbolCollector::shouldCollectSymbol(
           cast<NamedDecl>(RenameDecl), RenameDecl.getASTContext(),
-          SymbolCollector::Options(), CrossFile);
+          SymbolCollector::Options(), IsMainFileOnly);
   if (!IsIndexable) // If the symbol is not indexable, we disallow rename.
     return ReasonToReject::NonIndexable;
 
   if (!CrossFile) {
-    auto &ASTCtx = RenameDecl.getASTContext();
-    const auto &SM = ASTCtx.getSourceManager();
-    bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
-    bool DeclaredInMainFile = isInsideMainFile(RenameDecl.getBeginLoc(), SM);
-
     if (!DeclaredInMainFile)
       // We are sure the symbol is used externally, bail out early.
       return ReasonToReject::UsedOutsideFile;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70853.231525.patch
Type: text/x-patch
Size: 2624 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191129/33da0f16/attachment.bin>