rsmith added a comment.
================
Comment at: include/clang/AST/ASTContext.h:2490
/// it is not used.
- bool DeclMustBeEmitted(const Decl *D);
+ bool DeclMustBeEmitted(const Decl *D, bool WritingModule = false);
----------------
I think the name of this flag might be out of sync with its meaning; it looks
like it means "must this declaration be emitted when performing modular
codegen?"
================
Comment at: include/clang/Basic/Module.h:208
+ unsigned WithCodegen : 2;
+
----------------
Does this need to be two bits wide? Seems to only store true/false.
================
Comment at: include/clang/Driver/CC1Options.td:435-438
+def fmodule_codegen :
+ Flag<["-"], "fmodule-codegen">,
+ HelpText<"Generate code for uses of this module that assumes an explicit "
+ "object file will be built for the module">;
----------------
Maybe `module` -> `modules`, to match the other `-fmodules-*` flags controlling
various options.
================
Comment at: lib/AST/ASTContext.cpp:9020-9023
+ if (auto *Ext = getExternalSource())
+ if (Ext->hasExternalDefinitions(FD->getOwningModuleID()) ==
+ ExternalASTSource::EK_Never)
+ return true;
----------------
I think this `return true` is unreachable and can be deleted: if we get here
with `Linkage == GVA_DiscardableODR`, then the call to `hasExternalDefinitions`
in `GetGVALinkageForFunction` must have returned `EK_ReplyHazy`. (In the case
this is checking for, `GetGVALinkageForFunction` would return `GVA_StrongODR`,
so the check below will return `true` anyway.)
================
Comment at: lib/AST/ASTContext.cpp:9029
// Implicit template instantiations can also be deferred in C++.
return !isDiscardableGVALinkage(GetGVALinkageForFunction(FD));
}
----------------
Pass `Linkage` in here instead of recomputing it
================
Comment at: lib/AST/ExternalASTSource.cpp:33
+ExternalASTSource::hasExternalDefinitions(unsigned ID) {
+ return EK_ReplyHazy;
+}
----------------
You should add support for this function to
`clang::MultiplexExternalSemaSource` too.
================
Comment at: lib/Serialization/ASTWriterDecl.cpp:2215-2219
+ if (isRequiredDecl(D, Context, WritingModule, false))
EagerlyDeserializedDecls.push_back(ID);
+ else if (Context.getLangOpts().ModularCodegen && WritingModule &&
+ isRequiredDecl(D, Context, true, true))
+ ModularCodegenDecls.push_back(ID);
----------------
I suspect we'll want to seriously prune back the contents of
`EagerlyDeserializedDecls` for the modular codegen case at some point, but we
don't need to do that in this change.
https://reviews.llvm.org/D28845
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
