================
@@ -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

Reply via email to