Author: Sam McCall Date: 2023-07-21T23:30:31+02:00 New Revision: 290a98c7b00899b6aba0fc892e8f29fecc00a82e
URL: https://github.com/llvm/llvm-project/commit/290a98c7b00899b6aba0fc892e8f29fecc00a82e DIFF: https://github.com/llvm/llvm-project/commit/290a98c7b00899b6aba0fc892e8f29fecc00a82e.diff LOG: [clangd] Allow indexing of __reserved_names outside system headers The special handling for these names was added in https://github.com/llvm/llvm-project/commit/055d8090d1d5137dab88533995e0c5d9b5390c28 and the motivation was the C++ standard library. It turns out some projects like using these names anyway, in particular the linux kernel. D153946 proposed making this a config option, but there are some implementation issues with the config system. As an alternative, this patch tweaks the heuristic so we only drop these symbols in system headers. This does the right thing for linux and the C++ standard library, at least. Fixes https://github.com/clangd/clangd/issues/1680 Fixes https://github.com/llvm/llvm-project/issues/63862 Differential Revision: https://reviews.llvm.org/D155381 Added: Modified: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 214991a0674cc6..e9522171d70b75 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -516,7 +516,8 @@ bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND, 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; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index 0a890c4d5760a6..fe0de1925ecf2f 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -88,6 +88,7 @@ class SymbolCollector : public index::IndexDataConsumer { 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 diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp index 32a47c5ebfe865..774dcb8c60d1a1 100644 --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -304,10 +304,12 @@ class SymbolCollectorTest : public ::testing::Test { 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 diff erent 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 @@ TEST_F(SymbolCollectorTest, Origin) { 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, NoCrashOnObjCMethodCStyleParam) { 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()); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
