xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.
================
Comment at: test/Analysis/Inputs/externalFnMap.txt:1
+_Z7h_chaini@x86_64 xtu-chain.cpp.ast
+_ZN4chns4chf2Ei@x86_64 xtu-chain.cpp.ast
----------------
NoQ wrote:
> 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.
Good idea! We will add a test for that.
================
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:1
+//===- ClangCmdlineArchExtractor.cpp ------------------------------------===//
+//
----------------
NoQ wrote:
> 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.
We had the same idea recently. Next version of this patch will do this by
parsing the clang cc1 command line (gathered by -###).
================
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:72
+
+ return 0;
+}
----------------
danielmarjamaki wrote:
> EXIT_SUCCESS is also possible however I guess that is 0 on all
> implementations.
Other clang tools return 0.
================
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:192
+ lockedWrite(BuildDir + "/definedFns.txt", DefinedFuncsStr.str());
+ std::ostringstream CFGStr;
+ for (auto &Entry : CG) {
----------------
a.sidorin wrote:
> It is not 'CFG'Str, it should be 'CallGraph'Str. Sorry for this issue.
That part is removed from this version of the patch. It wasn't used anywhere.
================
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:257
+ Tool.run(newFrontendActionFactory<MapFunctionNamesAction>().get());
+}
----------------
danielmarjamaki wrote:
> no return.
That is not a problem for C++ programs, but added it.
https://reviews.llvm.org/D30691
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
