sammccall marked 8 inline comments as done.
sammccall added a comment.
> rarely happens anyway
citation needed.
While this is true today, it seems fairly likely that async preambles is going
to flip the dominant AST reuse from diagnostics -> read to read -> diagnostics.
This change would make the builds 20% faster for the cache misses on AST read
(~30%), at the cost of rebuilding when diagnostics would hit (~0%).
If those probabilities flip, this becomes a really bad trade.
Let's wait a couple of weeks until we have data from production.
================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:796
[](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
S.runWithAST("touchAST", FooCpp, [](Expected<InputsAndAST> IA) {
// Make sure the AST was actually built.
----------------
kadircet wrote:
> is it worth asserting here (or in a new test) that AST.diags() is empty,
> whereas it is non-empty below ? (ofc, after introducing some diags to the
> code)
I don't think we want to *guarantee* when they're present/absent with
runWithAST, this is an implementation choice. Added a comment to make this
contract explicit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81069/new/
https://reviews.llvm.org/D81069
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
