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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
