[PATCH] D155381: [clangd] Allow indexing of __reserved_names outside system headers

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG290a98c7b008: [clangd] Allow indexing of __reserved_names outside system headers (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D155381?vs=540734&id=543067#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155381/new/

https://reviews.llvm.org/D155381

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -304,10 +304,12 @@
         Args, Factory->create(), Files.get(),
         std::make_shared<PCHContainerOperations>());
 
-    InMemoryFileSystem->addFile(TestHeaderName, 0,
-                                llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-    InMemoryFileSystem->addFile(TestFileName, 0,
-                                llvm::MemoryBuffer::getMemBuffer(MainCode));
+    // Multiple calls to runSymbolCollector with different contents will fail
+    // to update the filesystem! Why are we sharing one across tests, anyway?
+    EXPECT_TRUE(InMemoryFileSystem->addFile(
+        TestHeaderName, 0, llvm::MemoryBuffer::getMemBuffer(HeaderCode)));
+    EXPECT_TRUE(InMemoryFileSystem->addFile(
+        TestFileName, 0, llvm::MemoryBuffer::getMemBuffer(MainCode)));
     Invocation.run();
     Symbols = Factory->Collector->takeSymbols();
     Refs = Factory->Collector->takeRefs();
@@ -1788,6 +1790,8 @@
   runSymbolCollector("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            Field(&Symbol::Origin, SymbolOrigin::Static)));
+  InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
+  CollectorOpts.CollectMacro = true;
   runSymbolCollector("#define FOO", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            Field(&Symbol::Origin, SymbolOrigin::Static)));
@@ -1941,17 +1945,25 @@
 
 TEST_F(SymbolCollectorTest, Reserved) {
   const char *Header = R"cpp(
+    #pragma once
     void __foo();
     namespace _X { int secret; }
   )cpp";
 
   CollectorOpts.CollectReserved = true;
-  runSymbolCollector("", Header);
+  runSymbolCollector(Header, "");
   EXPECT_THAT(Symbols, UnorderedElementsAre(qName("__foo"), qName("_X"),
                                             qName("_X::secret")));
 
   CollectorOpts.CollectReserved = false;
-  runSymbolCollector("", Header); //
+  runSymbolCollector(Header, "");
+  EXPECT_THAT(Symbols, UnorderedElementsAre(qName("__foo"), qName("_X"),
+                                            qName("_X::secret")));
+
+  // Ugly: for some reason we reuse the test filesystem across tests.
+  // You can't overwrite the same filename with new content!
+  InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
+  runSymbolCollector("#pragma GCC system_header\n" + std::string(Header), "");
   EXPECT_THAT(Symbols, IsEmpty());
 }
 
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -88,6 +88,7 @@
     bool CollectMainFileRefs = false;
     /// Collect symbols with reserved names, like __Vector_base.
     /// This does not currently affect macros (many like _WIN32 are important!)
+    /// This only affects system headers.
     bool CollectReserved = false;
     /// If set to true, SymbolCollector will collect doc for all symbols.
     /// Note that documents of symbols being indexed for completion will always
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -516,7 +516,8 @@
   if (isPrivateProtoDecl(ND))
     return false;
   if (!Opts.CollectReserved &&
-      (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext())))
+      (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext())) &&
+      ASTCtx.getSourceManager().isInSystemHeader(ND.getLocation()))
     return false;
 
   return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155381.543067.patch
Type: text/x-patch
Size: 3896 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230721/937bba3d/attachment-0001.bin>