================ @@ -1181,8 +1183,9 @@ class Preprocessor { public: Preprocessor(const PreprocessorOptions &PPOpts, DiagnosticsEngine &diags, - const LangOptions &LangOpts, SourceManager &SM, - HeaderSearch &Headers, ModuleLoader &TheModuleLoader, + const LangOptions &LangOpts, const CodeGenOptions &CGOPts, ---------------- benlangmuir wrote:
Having the `Preprocessor` keep a `CodeGenOptions` isn't technically a layering violation, but it feels like a bit of code smell to me. Looking through this PR, it seems the only places that actually use this are 1. `clang::InitializePreprocessor`: was there a reason you removed the `CodeGenOptions` parameter? That seems like a better design to me to have the preprocessor be independent of code gen opts after initialization. 2. `PCHValidator::ReadCodeGenOptions`: what if we stored the `CodeGenOptions` on the `ASTReader` instead? That seems to align better with the fact we are doing this in order to (de)serialize and check these options. 3. `ASTWriter`. Similarly, I'd rather have the ASTWriter keep the codegen opts not the preprocessor. https://github.com/llvm/llvm-project/pull/146422 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits