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@x86_64 xtu-chain.cpp.ast
+_ZN4chns4chf2Ei@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
