[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yeah, of course, ideally sticking this into scan-build, one way or another, would be great. I understand that it'd require quite a bit of communication with Laszlo. Otherwise we're just duplicating a whole lot of things.

Thanks for digging into this, guys. Really.



================
Comment at: lib/AST/ASTContext.cpp:1529-1530
+            iterateContextDecls(TU, MangledFnName, MangleCtx)) {
+    llvm::errs() << "Importing function " << MangledFnName << " from "
+                 << ASTFileName << "\n";
+    // FIXME: Refactor const_cast
----------------
These debug prints should be removed, i guess.


================
Comment at: lib/AST/ASTImporter.cpp:3222
     
-      if (FunctionDecl *FoundFunction = dyn_cast<FunctionDecl>(FoundDecls[I])) {
+      if (auto *FoundFunction = dyn_cast<FunctionDecl>(FoundDecls[I])) {
         if (FoundFunction->hasExternalFormalLinkage() &&
----------------
We could probably commit the `auto`-related changes separately in order to keep this patch clean.


================
Comment at: test/Analysis/Inputs/externalFnMap.txt:1
+_Z7h_chaini at x86_64 xtu-chain.cpp.ast
+_ZN4chns4chf2Ei at x86_64 xtu-chain.cpp.ast
----------------
These tests use a pre-made external function map.

Are you willing to add tests for generating function maps?

That'd be useful, in my opinion, because it'd actually tell people how to run the whole thing.


================
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:1
+//===- ClangCmdlineArchExtractor.cpp ------------------------------------===//
+//
----------------
Do we really intend to keep this as a tool? I suspect obtaining the architecture could be done much easier by parsing the run-line in python scripts, we were just in too much rush to consider this possibility, and calling a whole tool for just that sounds like an overkill. I think it would be great to simplify this out.

Additionally, this whole architecture hassle kicks in only rarely. It is only important to know the architecture because some projects have the same file compiled for different architectures (we used to have it in Android which has binaries that are compiled for both host machine and target machine, but for most projects just having a mangled name is already good enough). So we can delay this discussion by splitting this out of the initial patch, if you want, but that's extra work, of course, so please take the above as more of a mumble than of a request.


https://reviews.llvm.org/D30691