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

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

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.



================
Comment at: include/clang/Basic/AllDiagnostics.h:20
 #include "clang/AST/CommentDiagnostic.h"
+#include "clang/CrossTU/CrossTUDiagnostic.h"
 #include "clang/Analysis/AnalysisDiagnostic.h"
----------------
Import file order?


================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:42
+/// Note that this class also implements caching.
+class CrossTranslationUnit {
+public:
----------------
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.




================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:45
+
+/// Recursively visit the funtion decls of a DeclContext, and looks up a
+/// function based on mangled name.
----------------
"visit**s** (...) and looks up" or "visit (...) and look up"


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:46
+/// Recursively visit the funtion decls of a DeclContext, and looks up a
+/// function based on mangled name.
+const FunctionDecl *
----------------
If we are using //USR// for lookup, then why does this look up "based on mangled name"?


================
Comment at: tools/CMakeLists.txt:25
   add_clang_subdirectory(scan-view)
+  add_clang_subdirectory(clang-func-mapping)
 endif()
----------------
The other "blocks" in this file look //somewhat// in alphabetic order, perhaps this should be added a few lines above?


https://reviews.llvm.org/D34512