ilya-biryukov added inline comments.
================ Comment at: include/clang/Basic/DiagnosticLexKinds.td:432 +def err_pp_including_mainfile_for_preamble : Error< + "main file cannot be included recursively for preamble">, DefaultFatal; def err_pp_empty_filename : Error<"empty filename">; ---------------- NIT: maybe rephrase `for preamble` to `when building a preamble`? "for preamble" might be a bit confusing. ================ Comment at: include/clang/Lex/Preprocessor.h:391 } PreambleConditionalStack; + bool PreambleGenerationFailed = false; ---------------- nik wrote: > 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. if `AllowPCHWithCompilerErrors=true` is set to true, why not simply pretend the include was not found? That would still generate the preamble, but users have the error message to see that something went wrong. `c-index-test` produces the errors, so we can check the error is present in the output. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53866/new/ https://reviews.llvm.org/D53866 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
