[PATCH] D34444: Teach codegen to work in incremental processing mode.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  In that case, I see two problems, one major and one potentially major.

The major problem is that, as Richard alludes to, you need to make sure you emit any on-demand definitions that Sema registered with CodeGen during the initial CGM's lifetime but used in later CGMs.  Probably the easiest way of doing that is to eliminate CodeGenModule's dependence on having on-demand definitions pre-registered with it: that is, if it emits a reference to a declaration whose definition already exists in the AST, CodeGenModule should use that definition even if e.g. HandleTopLevelDecl has not been called.  Calling HandleTopLevelDecl would only be necessary if the definition is discovered/instantiated after code is generated for the use.  I see little advantage to the current rule, and doing this would also be a nice quality-of-life improvement for various out-of-tree projects (including Swift, but IIRC there are several others) which deserialize references to a complete AST, and which currently have to walk the definition and recursively register any definitions used within.

The potentially major problem is that it is not possible in general to automatically break up a single translation unit into multiple translation units, for two reasons.  The big reason is that there is no way to correctly handle entities with non-external linkage that are referenced from two parts of the translation unit without implicitly finding some way to promote them to external linkage, which might open a huge can of worms if you have multiple "outer" translation units.  The lesser reason is that the prefix of a valid translation unit is not necessarily a valid translation unit: for example, a static or inline function can be defined at an arbitrary within the translation unit, i.e. not necessarily before its first use.  But if your use case somehow defines away these problems, this might be fine.

As a minor request, if you are going to make HandleTranslationUnit no longer the final call on CodeGenerator, please either add a new method that *is* a guaranteed final call or add a new method that does the whole "end a previous part of the translation unit and start a new one" step.



================
Comment at: lib/CodeGen/ModuleBuilder.cpp:328
 
+llvm::Module *CodeGenerator::StartModule(const std::string& ModuleName,
+                                         llvm::LLVMContext& C) {
----------------
Why does this one take a const std::string & instead of a StringRef?


Repository:
  rL LLVM

https://reviews.llvm.org/D34444