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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a subscriber: karies.
v.g.vassilev added a comment.

@rjmccall, thanks for the prompt and thorough reply.

In https://reviews.llvm.org/D34444#793311, @rjmccall wrote:

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




  This is a very accurate diagnosis which took us 5 years to discover on an empirical basis ;)

> 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.



  We bring the CGM state to the subsequent CGMs. See https://github.com/vgvassilev/clang/blob/cling-patches/lib/CodeGen/ModuleBuilder.cpp#L138-L160

> 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.



  We do not have an multiple 'outer' translation units. We have just one ever growing TU (which probably invalidates my previous statement that we have a distinct TUs) which we send to the RuntimeDyLD allowing only JIT to resolve symbols from it.  We aid the JIT when resolving symbols with internal linkage by changing all internal linkage to external (We haven't seen issues with that approach).

>   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.



  If we end up with a module containing no definition of a symbol and such is required, then we complain. So indeed we are defining away this issue.
   

> 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.



  We can have this but it would be a copy of `HandleTranslationUnit`.  The `StartModule` interface is the antagonist routine to `ReleaseModule`. If you prefer we could merge `StartModule` into `ReleaseModule` adding a flag (or reading the value of `isIncrementalProcessingEnabled`).

(Thanks @karies for helping!)


Repository:
  rL LLVM

https://reviews.llvm.org/D34444