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 [email protected] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
