This revision was automatically updated to reflect the committed changes.
Closed by commit rL358571: [clangd] Include insertion: require header guards,
drop other heuristics, treat… (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
sammccall updated this revision to Diff 195519.
sammccall marked 3 inline comments as done.
sammccall added a comment.
address review comments
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60316/new/
https://reviews.llvm.org/D60316
Files:
ioeric requested changes to this revision.
ioeric added a comment.
This revision now requires changes to proceed.
in case you missed this patch :)
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60316/new/
https://reviews.llvm.org/D60316
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:157
+ return Canonical.str();
+else if (Canonical != Filename)
+ return toURI(SM, Canonical, Opts);
nit: no need for `else`?
Comment at:
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: unittests/clangd/FileIndexTests.cpp:214
TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+ TestTU TU;
I suspect we can replace most of these tests with TestTU -
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
We do have some reports of include insertion behaving badly in some
codebases. Requiring header guards