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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to