nik marked 2 inline comments as done. nik added inline comments.
================ Comment at: include/clang/Lex/Preprocessor.h:391 } PreambleConditionalStack; + bool PreambleGenerationFailed = false; ---------------- 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? ================ Comment at: lib/Lex/PPDirectives.cpp:1945 + // Generate a fatal error to skip further processing. + Diag(FilenameTok.getLocation(), diag::err_pp_error_opening_file) << "" + << ""; ---------------- ilya-biryukov wrote: > Maybe add a new error or find a more appropriate existing one to reuse? > "Error opening file", especially without any arguments does not provide > enough context to figure out what went wrong. > Maybe add a new error or find a more appropriate existing one to reuse? As stated above, introducing a new diagnostic that the user will never face feels wrong, but that's just a preference. If you prefer a dedicated diagnostics, that's fine for me. Checking the existing ones, there are not many fatal errors to choose from: err_pp_file_not_found err_pp_through_header_not_found err_pp_through_header_not_seen err_pp_pragma_hdrstop_not_seen err_pp_error_opening_file The last one looks the best for me. > "Error opening file", especially without any arguments does not provide > enough context to figure out what went wrong. I've added some arguments which might be useful for debugging. 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
