[PATCH] D34512: Add preliminary Cross Translation Unit support library

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 10 02:26:04 PDT 2017


xazax.hun added a comment.

In https://reviews.llvm.org/D34512#836831, @whisperity wrote:

> Apart from those in the in-line comments, I have a question: how safe is this library to `Release` builds? I know this is only a submodule dependency for the "real deal" in https://reviews.llvm.org/D30691, but I have seen some asserts that "imported function should already have a body" and such.
>
> Will the static analyzer handle these errors gracefully and fall back to "function is unknown, let's throw my presumptions out of the window" or will it bail away? I'm explicitly thinking of the assert-lacking `Release` build.


The basic idea is that, if a function already have a body in the current translation unit and you still want to import it from somewhere else, it might be a programmer error, so we do not handle this gracefully.



================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:42
+/// Note that this class also implements caching.
+class CrossTranslationUnit {
+public:
----------------
whisperity wrote:
> Does the name of this class make sense? If I say
> 
> 
> ```
> CrossTranslationUnit *ctuptr = new CrossTranslationUnit(...);
> ```
> 
> did I construct a new //CrossTranslationUnit//? What even **is** a //CrossTranslationUnit//? Other class names, such as `ASTImporter`, `DiagOpts`, and `FrontendAction`, etc. somehow feel better at ~~transmitting~~ reflecting upon their meaning in their name.
> 
> 
What would you think about `CrossTranslationUnitContext`?

The functionality of this class is have all the relevant data required for doing Cross TU lookups and also provide the interface for that. 


https://reviews.llvm.org/D34512





More information about the cfe-commits mailing list