[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
This revision was automatically updated to reflect the committed changes. Closed by commit rG64366d4935d3: [clangd] Rollforward include-cleaner library usage in symbol collector. (authored by VitaNuo). Changed prior to commit: https://reviews.llvm.org/D156659?vs=556135&id=556396#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexActionTests.cpp 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 @@ -14,6 +14,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" @@ -1554,6 +1555,8 @@ // Move overloads have special handling. template T&& move(_T&& __value); template _O move(_I, _I, _O); +template _O move( + _T&&, _O, _O, _I); } )cpp", /*Main=*/""); @@ -1565,7 +1568,8 @@ includeHeader("")), // Parameter names are demangled. AllOf(labeled("move(T &&value)"), includeHeader("")), - AllOf(labeled("move(I, I, O)"), includeHeader(""; + AllOf(labeled("move(I, I, O)"), includeHeader("")), + AllOf(labeled("move(T &&, O, O, I)"), includeHeader(""; } TEST_F(SymbolCollectorTest, IWYUPragma) { @@ -1592,7 +1596,7 @@ includeHeader("\"the/good/header.h\""; } -TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) { +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; const std::string Header = R"cpp(#pragma once #include "exporter.h" @@ -1978,6 +1982,24 @@ qName("A"), hasKind(clang::index::SymbolKind::Concept; } +TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once +struct Foo; +#include "full.h" +)cpp"; + auto FullFile = testPath("full.h"); + InMemoryFileSystem->addFile(FullFile, 0, + llvm::MemoryBuffer::getMemBuffer(R"cpp( +#pragma once +struct Foo {};)cpp")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(FullFile).toString() + << *Symbols.begin(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp === --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -8,11 +8,15 @@ #include "Headers.h" #include "TestFS.h" +#include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -40,6 +44,11 @@ return toUri(Path) == URI; } +MATCHER_P(includeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} + ::testing::Matcher includesAre(const std::vector &Includes) { return ::testing::Field(&IncludeGraphNode::DirectIncludes, @@ -312,6 +321,26 @@ EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar"))); EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"; } + +TEST_F(IndexActionTest, SymbolFromCC) { + std::string MainFilePath = testPath("main.cpp"); + addFile(MainFilePath, R"cpp( + #include "main.h" + void foo() {} + )cpp"); + addFile(testPath("main.h"), R"cpp( + #pragma once + void foo(); + )cpp"); + Opts.FileFilter = [](const SourceManager &SM, FileID F) { +return !SM.getFileEntryRefForID(F)->getName().endswith("main.h"); + }; + IndexFileIn IndexFile = runIndexingAction(MainFilePath, {"-std=c++14"}); + EXPECT_THAT(*IndexFile.Symbols, + UnorderedElementsAre(AllOf( + hasName("foo"), + includeHeader(URI::create(testPath("main.h")).toString(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-to
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo updated this revision to Diff 556135. VitaNuo marked an inline comment as done. VitaNuo added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexActionTests.cpp clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp clang-tools-extra/include-cleaner/lib/FindHeaders.cpp Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp === --- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp @@ -125,7 +125,7 @@ if (FD->getNumParams() == 1) // move(T&& t) return tooling::stdlib::Header::named(""); -if (FD->getNumParams() == 3) +if (FD->getNumParams() == 3 || FD->getNumParams() == 4) // move(InputIt first, InputIt last, OutputIt dest); return tooling::stdlib::Header::named(""); } else if (FName == "remove") { Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp === --- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -14,6 +14,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" @@ -1554,6 +1555,8 @@ // Move overloads have special handling. template T&& move(_T&& __value); template _O move(_I, _I, _O); +template _O move( + _T&&, _O, _O, _I); } )cpp", /*Main=*/""); @@ -1565,7 +1568,8 @@ includeHeader("")), // Parameter names are demangled. AllOf(labeled("move(T &&value)"), includeHeader("")), - AllOf(labeled("move(I, I, O)"), includeHeader(""; + AllOf(labeled("move(I, I, O)"), includeHeader("")), + AllOf(labeled("move(T &&, O, O, I)"), includeHeader(""; } TEST_F(SymbolCollectorTest, IWYUPragma) { @@ -1592,7 +1596,7 @@ includeHeader("\"the/good/header.h\""; } -TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) { +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; const std::string Header = R"cpp(#pragma once #include "exporter.h" @@ -1978,6 +1982,24 @@ qName("A"), hasKind(clang::index::SymbolKind::Concept; } +TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once +struct Foo; +#include "full.h" +)cpp"; + auto FullFile = testPath("full.h"); + InMemoryFileSystem->addFile(FullFile, 0, + llvm::MemoryBuffer::getMemBuffer(R"cpp( +#pragma once +struct Foo {};)cpp")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(FullFile).toString() + << *Symbols.begin(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp === --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -8,11 +8,15 @@ #include "Headers.h" #include "TestFS.h" +#include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -40,6 +44,11 @@ return toUri(Path) == URI; } +MATCHER_P(includeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} + ::testing::Matcher includesAre(const std::vector &Includes) { return ::testing::Field(&IncludeGraphNode::DirectIncludes, @@ -312,6 +321,26 @@ EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar"))); EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"; } + +TEST_F(IndexActionTest, SymbolFromCC) { + std::string MainFilePath = testPath("main.cpp"); + addFile(MainFilePath, R"cpp( + #include "main.h" + void foo() {} + )cpp"); + addFile(testPath("main.h"), R"cpp( + #pragma once
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo marked 2 inline comments as done. VitaNuo added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:919 + if (IncludeHeader.empty()) +HeaderFileURIs->getIncludeHeader(FID); + kadircet wrote: > i guess LHS of the assignment got dropped by mistake? Sorry, juggling unconnected stuff makes me inattentive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:828 - tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX; -if (LangOpts.C11) - Lang = tooling::stdlib::Lang::C; sorry i got confused, this also works for ObjC, not just ObjC++. we set C-like features for ObjectiveC. can you just restore as-is? Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:919 + if (IncludeHeader.empty()) +HeaderFileURIs->getIncludeHeader(FID); + i guess LHS of the assignment got dropped by mistake? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo updated this revision to Diff 556130. VitaNuo marked 2 inline comments as done. VitaNuo added a comment. Address the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexActionTests.cpp clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp clang-tools-extra/include-cleaner/lib/FindHeaders.cpp Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp === --- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp @@ -125,7 +125,7 @@ if (FD->getNumParams() == 1) // move(T&& t) return tooling::stdlib::Header::named(""); -if (FD->getNumParams() == 3) +if (FD->getNumParams() == 3 || FD->getNumParams() == 4) // move(InputIt first, InputIt last, OutputIt dest); return tooling::stdlib::Header::named(""); } else if (FName == "remove") { Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp === --- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -14,6 +14,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" @@ -1554,6 +1555,8 @@ // Move overloads have special handling. template T&& move(_T&& __value); template _O move(_I, _I, _O); +template _O move( + _T&&, _O, _O, _I); } )cpp", /*Main=*/""); @@ -1565,7 +1568,8 @@ includeHeader("")), // Parameter names are demangled. AllOf(labeled("move(T &&value)"), includeHeader("")), - AllOf(labeled("move(I, I, O)"), includeHeader(""; + AllOf(labeled("move(I, I, O)"), includeHeader("")), + AllOf(labeled("move(T &&, O, O, I)"), includeHeader(""; } TEST_F(SymbolCollectorTest, IWYUPragma) { @@ -1592,7 +1596,7 @@ includeHeader("\"the/good/header.h\""; } -TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) { +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; const std::string Header = R"cpp(#pragma once #include "exporter.h" @@ -1978,6 +1982,24 @@ qName("A"), hasKind(clang::index::SymbolKind::Concept; } +TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once +struct Foo; +#include "full.h" +)cpp"; + auto FullFile = testPath("full.h"); + InMemoryFileSystem->addFile(FullFile, 0, + llvm::MemoryBuffer::getMemBuffer(R"cpp( +#pragma once +struct Foo {};)cpp")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(FullFile).toString() + << *Symbols.begin(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp === --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -8,11 +8,15 @@ #include "Headers.h" #include "TestFS.h" +#include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -40,6 +44,11 @@ return toUri(Path) == URI; } +MATCHER_P(includeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} + ::testing::Matcher includesAre(const std::vector &Includes) { return ::testing::Field(&IncludeGraphNode::DirectIncludes, @@ -312,6 +321,26 @@ EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar"))); EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"; } + +TEST_F(IndexActionTest, SymbolFromCC) { + std::string MainFilePath = testPath("main.cpp"); + addFile(MainFilePath, R"cpp( + #include "main.h" + void foo() {} + )cpp"); + addFile(testPath("main.h"), R"cpp( + #pragma o
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:912 +if (Directives & Symbol::Import) { + if (auto IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); + !IncludeHeader.empty()) { VitaNuo wrote: > kadircet wrote: > > we should keep the `getStdHeaders` logic for objc > `getStdHeaders` returns empty string for Obj-C, are you sure you meant it? it reads a little bit weird, but it still actually works, for objective-c++ to be more specific. as we'll have `LangOpts.CPlusPlus` set for objc++ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:912 +if (Directives & Symbol::Import) { + if (auto IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); + !IncludeHeader.empty()) { kadircet wrote: > we should keep the `getStdHeaders` logic for objc `getStdHeaders` returns empty string for Obj-C, are you sure you meant it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo updated this revision to Diff 556004. VitaNuo marked 5 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexActionTests.cpp clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp clang-tools-extra/include-cleaner/lib/FindHeaders.cpp Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp === --- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp @@ -125,7 +125,7 @@ if (FD->getNumParams() == 1) // move(T&& t) return tooling::stdlib::Header::named(""); -if (FD->getNumParams() == 3) +if (FD->getNumParams() == 3 || FD->getNumParams() == 4) // move(InputIt first, InputIt last, OutputIt dest); return tooling::stdlib::Header::named(""); } else if (FName == "remove") { Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp === --- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -14,6 +14,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" @@ -1554,6 +1555,8 @@ // Move overloads have special handling. template T&& move(_T&& __value); template _O move(_I, _I, _O); +template _O move( + _T&&, _O, _O, _I); } )cpp", /*Main=*/""); @@ -1565,7 +1568,8 @@ includeHeader("")), // Parameter names are demangled. AllOf(labeled("move(T &&value)"), includeHeader("")), - AllOf(labeled("move(I, I, O)"), includeHeader(""; + AllOf(labeled("move(I, I, O)"), includeHeader("")), + AllOf(labeled("move(T &&, O, O, I)"), includeHeader(""; } TEST_F(SymbolCollectorTest, IWYUPragma) { @@ -1592,7 +1596,7 @@ includeHeader("\"the/good/header.h\""; } -TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) { +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; const std::string Header = R"cpp(#pragma once #include "exporter.h" @@ -1978,6 +1982,24 @@ qName("A"), hasKind(clang::index::SymbolKind::Concept; } +TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once +struct Foo; +#include "full.h" +)cpp"; + auto FullFile = testPath("full.h"); + InMemoryFileSystem->addFile(FullFile, 0, + llvm::MemoryBuffer::getMemBuffer(R"cpp( +#pragma once +struct Foo {};)cpp")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(FullFile).toString() + << *Symbols.begin(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp === --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -8,11 +8,15 @@ #include "Headers.h" #include "TestFS.h" +#include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -40,6 +44,11 @@ return toUri(Path) == URI; } +MATCHER_P(includeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} + ::testing::Matcher includesAre(const std::vector &Includes) { return ::testing::Field(&IncludeGraphNode::DirectIncludes, @@ -312,6 +321,26 @@ EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar"))); EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"; } + +TEST_F(IndexActionTest, SymbolFromCC) { + std::string MainFilePath = testPath("main.cpp"); + addFile(MainFilePath, R"cpp( + #include "main.h" + void foo() {} + )cpp"); + addFile(testPath("main.h"), R"cpp( + #prag
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:840 -if (S->Scope == "std::" && S->Name == "move") { - if (!S->Signature.contains(',')) -return ""; - return ""; -} - -if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang)) - if (auto Header = StdSym->header()) - return Header->name(); -return ""; + auto [It, Inserted] = SymbolProviders.try_emplace(S.ID); + auto Headers = can you add a comment here saying, `We update providers for a symbol with each occurence, as SymbolCollector might run while parsing, rather than at the end of a translation unit. Hence we see more and more redecls over time.` Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:912 +if (Directives & Symbol::Import) { + if (auto IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); + !IncludeHeader.empty()) { we should keep the `getStdHeaders` logic for objc Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:934 +if (auto Canonical = +HeaderFileURIs->mapCanonical(H.physical()->getName()); +!Canonical.empty()) can you add a `// FIXME: Get rid of this once include-cleaner has support for system headers.` to this branch Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:937 + SpellingIt->second = Canonical; +else if (tooling::isSelfContainedHeader(H.physical(), SM, +PP->getHeaderSearchInfo())) again a comment saying `For physical files, prefer URIs as spellings might change depending on the translation unit.` Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:127 +if (FD->getNumParams() == 3 || FD->getNumParams() == 4) // move(InputIt first, InputIt last, OutputIt dest); return tooling::stdlib::Header::named(""); can you also add comment `move(ExecutionPolicy&& policy, ForwardIt1 first, ForwardIt1 last, ForwardIt2 d_first );` and move this change into a separate patch with a test case in clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp? Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc:365 +// Remove them when the cause(s) are identified. +SYMBOL(div, std::, ) +SYMBOL(abort, std::, ) can you move this into a separate patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156659/new/ https://reviews.llvm.org/D156659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.
VitaNuo created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. VitaNuo requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added projects: clang, clang-tools-extra. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D156659 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang-tools-extra/clangd/index/SymbolCollector.h clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexActionTests.cpp clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp clang-tools-extra/include-cleaner/lib/FindHeaders.cpp clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc Index: clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc === --- clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc +++ clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc @@ -360,6 +360,11 @@ SYMBOL(make_index_sequence, std::, ) SYMBOL(make_integer_sequence, std::, ) +// generated symbol map is missing these symbols. +// Remove them when the cause(s) are identified. +SYMBOL(div, std::, ) +SYMBOL(abort, std::, ) + // The std::placeholder symbols (_1, ..., _N) are listed in the cppreference // placeholder.html, but the index only contains a single entry with "_1, _2, ..., _N" // text, which are not handled by the script. Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp === --- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp @@ -123,7 +123,7 @@ if (FD->getNumParams() == 1) // move(T&& t) return tooling::stdlib::Header::named(""); -if (FD->getNumParams() == 3) +if (FD->getNumParams() == 3 || FD->getNumParams() == 4) // move(InputIt first, InputIt last, OutputIt dest); return tooling::stdlib::Header::named(""); } else if (FName == "remove") { Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp === --- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -14,6 +14,7 @@ #include "index/SymbolCollector.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Index/IndexingAction.h" #include "clang/Index/IndexingOptions.h" @@ -1554,6 +1555,8 @@ // Move overloads have special handling. template T&& move(_T&& __value); template _O move(_I, _I, _O); +template _O move( + _T&&, _O, _O, _I); } )cpp", /*Main=*/""); @@ -1565,7 +1568,8 @@ includeHeader("")), // Parameter names are demangled. AllOf(labeled("move(T &&value)"), includeHeader("")), - AllOf(labeled("move(I, I, O)"), includeHeader(""; + AllOf(labeled("move(I, I, O)"), includeHeader("")), + AllOf(labeled("move(T &&, O, O, I)"), includeHeader(""; } TEST_F(SymbolCollectorTest, IWYUPragma) { @@ -1592,7 +1596,7 @@ includeHeader("\"the/good/header.h\""; } -TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) { +TEST_F(SymbolCollectorTest, IWYUPragmaExport) { CollectorOpts.CollectIncludePath = true; const std::string Header = R"cpp(#pragma once #include "exporter.h" @@ -1978,6 +1982,24 @@ qName("A"), hasKind(clang::index::SymbolKind::Concept; } +TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) { + CollectorOpts.CollectIncludePath = true; + const std::string Header = R"cpp(#pragma once +struct Foo; +#include "full.h" +)cpp"; + auto FullFile = testPath("full.h"); + InMemoryFileSystem->addFile(FullFile, 0, + llvm::MemoryBuffer::getMemBuffer(R"cpp( +#pragma once +struct Foo {};)cpp")); + runSymbolCollector(Header, /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( + qName("Foo"), + includeHeader(URI::create(FullFile).toString() + << *Symbols.begin(); +} } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp === --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -8,11 +8,15 @@ #include "Headers.h" #include "TestFS.h" +#include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "cl