kadircet added inline comments.
================
Comment at: clangd/index/IndexAction.cpp:45
+ auto &Node = I->getValue();
+ if (auto Digest = digestFile(SM, FileID))
+ Node.Digest = std::move(*Digest);
----------------
ilya-biryukov wrote:
> What happens if we can't compute a digest for a file? Are we left with an
> uninitialized memory?
Yes, we'll be left with some random digest in the node.
But I believe it is as good as any sentinel value, this field is used for
consistency checks. Absence of digest implies we will always(well almost
always) have an inconsistent(~stale) version of the file in the current
structure and will re-trigger indexing action next time.
Therefore, having this digest initialized to some sentinel or random gibberish
has the same probability of colliding with the actual hash. So I believe it is
OK to leave it like that.
================
Comment at: clangd/index/IndexAction.cpp:109
CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
+ if (IncludeGraphCallback != nullptr)
+ CI.getPreprocessor().addPPCallbacks(
----------------
ilya-biryukov wrote:
> NIT: maybe remove `!= nullptr`? the callback is a function, not a pointer, so
> `nullptr` might be a bit confusing
yeah but this was the convention in the file, is it ok if I change the other
usages as well?
================
Comment at: unittests/clangd/IndexActionTests.cpp:126
+}
+
+} // namespace
----------------
ilya-biryukov wrote:
> Could we also test a few corner cases?
>
> - Self-includes (with and without include guards).
> - Skipped files (multiple includes of the same file with `#pragma once` or
> include guards)
>
I've added a self-include test with header guards, I don't think it is possible
to do that without a guard. Wouldn't it cause an infinite loop? I end up
getting abort with:
`/clangd-test/header.h:1:10: error: #include nested too deeply`
Did you mean something else?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54999/new/
https://reviews.llvm.org/D54999
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
