[PATCH] D83178: [clangd] Send EOF before resetting diagnostics consumer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

> can emit diagnostics at this callback (as mentioned in the comments).

I'm a bit puzzled about this. The case that the code handles and the comment refers to is if a check installs PPCallbacks and overrides EndSourceFile. And I believe the existing code is ok for this case: we don't detach those callbacks.

It didn't occur to me that EOF could be observed through the DiagnosticConsumer instead, and this patch does look like a reasonable fix for that. However ClangTidyDiagnosticConsumer doesn't override EndSourceFile nor even the destructor, so it doesn't seem to actually happen.

LLVM-include-order is hooking PPCallbacks::EndOfMainFile, i.e. the first case and the one I thought we were already handling, and I wouldn't expect changing the relative order of EOF and resetting the DC to matter for that.

So I believe you that it fixes something but I'd feel better having some idea of how :-)
(We have a test with a fake clang tidy check IIRC, I wonder if we can test this there)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83178