[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 11 20:35:48 PDT 2019
dexonsmith marked 2 inline comments as done.
dexonsmith added inline comments.
Comment at: clang/lib/Frontend/FrontendActions.cpp:182
+ CI.getFrontendOpts().BuildingImplicitModule &&
> jordan_rose wrote:
> > Why is this the condition, as opposed to just "do this for all modules, don't do it for PCHs"? And doesn't `BuildingImplicitModule` imply `isCompilingModule()`? (Note that `BuildingImplicitModule` probably isn't the same as the original condition `ImplicitModules`.)
> > (Note that BuildingImplicitModule probably isn't the same as the original condition ImplicitModules.)
> Nice catch; I ported the logic from `ASTWriter` incorrectly. I'll fix this.
> > Why is this the condition, as opposed to just "do this for all modules, don't do it for PCHs"? And doesn't BuildingImplicitModule imply isCompilingModule()?
> I was cargo-culting the logic for how `PCHGenerator::HandleTranslationUnit` sets the `Module` parameter to `ASTWriter::WriteAST`. I think that if I fix the above to match the original condition I'll need to check `isCompilingModule()`... but maybe `BuildingImplicitModule` is already `&&`-ing these together? I'll check.
Updated the patch to just use `BuildingImplicitModule`. The previous condition in `PCHGenerator::HandleTranslationUnit` seems to have been equivalent.
- Previously in `PCHGenerator::HandleTranslationUnit`, `WritingModule` would be non-null only if we're currently building a module, as opposed to writing out a PCH. The logic to write to the in-memory cache also checked if `LangOptions::ImplicitModules` was set.
- Now in `GenerateModuleAction::CreateASTConsumer` we check `BuildingImplicitModule` which is set in `compileModuleImpl` before executing the action.
I'm choosing this because it better matches the code around.
CHANGES SINCE LAST ACTION
More information about the cfe-commits