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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to