[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

[Nikolai Kosjar via Phabricator via cfe-commits]

nik marked 2 inline comments as done and an inline comment as not done.
nik added inline comments.


================
Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 
----------------
ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > There's a mechanism to handle preamble with errors, see `PreprocessorOpts::AllowPCHWithCompilerErrors`.
> > > Maybe rely on it and avoid adding a different one?
> > I'm not sure how to make use of AllowPCHWithCompilerErrors. It's clearly meant as an input/readonly option - do you suggest setting that one to "false" in case we detect the cyclic include (and later checking for it...)? That feels a bit hacky. Have you meant something else?
> We emit an error, so the preamble will **not** be emitted. 
> Unless the users specify `AllowPCHWithCompilerErrors`, in which case they've signed up for this anyway.
> 
> I propose to **not** add the new flag at all. Would that work?
As stated further above, no.

That's because for the libclang/c-index-test case, AllowPCHWithCompilerErrors=true - see clang_parseTranslationUnit_Impl. As such, the preamble will be generated/emitted as the second early return in PCHGenerator::HandleTranslationUnit is never hit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866