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