[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

[Jan Korous via Phabricator via cfe-commits]

jkorous accepted this revision.
jkorous added a comment.
This revision is now accepted and ready to land.

LGTM with just a nit about comment wording.

Thanks for the patch!



================
Comment at: clang/lib/Lex/PPDirectives.cpp:1875
+  // Stop further preprocessing if a fatal error has occurred. Any diagnostics
+  // we might have raised will not be visible.
+  if (ShouldEnter && Diags->hasFatalErrorOccurred())
----------------
vsapsai wrote:
> jkorous wrote:
> > I am not sure I understand this - does that mean that we are not displaying diagnostics that were produced before "now"?
> I can see how the wording can cause the confusion. But I'm not entirely sure it is misguiding, I've copy-pasted it from [Sema::InstantiatingTemplate::InstantiatingTemplate ](https://github.com/llvm-mirror/clang/blob/580f7daabc7696d50ad09d9643b2afeadbd387d8/lib/Sema/SemaTemplateInstantiate.cpp#L218-L220). Let me explain my reasoning in a different way to see if it makes sense. Entering a file is observable if it produces diagnostics or some other build artifact (object file in most cases). So when we encounter a fatal error, there is no visible indication of entering subsequent files. That's why we can skip entering those files: no difference in output and less work to do.
Thanks for the explanation! I was not sure whether "only subsequent" OR "both subsequent and some/all prior" raised diagnostics would be not visible (could be just my shitty English though).

Maybe something along this line "any eventual subsequent diagnostics will not be visible" would be more clear?


https://reviews.llvm.org/D48786