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



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

Reply via email to