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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.


================
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())
----------------
jkorous wrote:
> 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?
Curiously, the comment kinda means both. Some prior diagnostics might be invisible but in this case we care about subsequent diagnostics. I'll update the comment to make that more clear.


https://reviews.llvm.org/D48786