[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=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 &)"), 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 ) {
return ::testing::Field(::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 , 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:
[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 &)"), 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 ) {
return ::testing::Field(::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");
+
[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 &)"), 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 ) {
return ::testing::Field(::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");
+
[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 &)"), 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 ) {
return ::testing::Field(::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");
[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 &)"), 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
