[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:464
-
-  TU.AdditionalFiles["a.h"] = "";
-  TU.AdditionalFiles["b.h"] = "";
----------------
kadircet wrote:
> sammccall wrote:
> > Why are these tests deleted?
> they rely on the fact that clang-tidy checkers PPcallbacks are run with patched asts, but it is no longer the case, hence it becomes impossible to satisfy them.
> 
> currently we never replay includes with patched preambles, as ReplayPreamble bails out when there are no existing PPCallbacks and that's always the case. we can still try and test it via complicated means like enabling ClangdFeatures to register PPCallbacks, but it will be testing a feature that doesn't exist in practice + would be quite some work for just testing, so I'd rather leave it as-is (probably by adding a comment around ReplayPreamble::attach saying that we should test this once there are non-clang-tidy users).
Oops, I didn't scroll up fast enough (reviewing on phone lol). SGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109884/new/

https://reviews.llvm.org/D109884