vsk added a comment.
Looks good. Apart from some nitpicks, I'm happy with this. This isn't really my
area so it'd be good to get an explicit lgtm from one of the reviewers.
================
Comment at: tools/clang-import-test/CMakeLists.txt:7
+ clang-import-test.cpp
+ )
+
----------------
@beanz recently went on a crusade to add dependencies to intrinsics_gen to a
bunch of stuff. Chances are, this tool probably depends on intrinsics_gen, so
I'd look into that and add the dep if needed.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:306
+ std::transform(ImportCIs.begin(), ImportCIs.end(), UnownedCIs.begin(),
+ std::mem_fn(&std::unique_ptr<CompilerInstance>::get));
+ llvm::Expected<std::unique_ptr<CompilerInstance>> ExpressionCI =
----------------
This transform smells a little strange. I'd rather see
`ArrayRef<std::unique_ptr<...>>` used everywhere, through a typedef/using-decl
if necessary.
================
Comment at: tools/clang-import-test/clang-import-test.cpp:310
+ if (auto E = ExpressionCI.takeError()) {
+ llvm::errs() << (llvm::toString(std::move(E)));
+ exit(-1);
----------------
There are redundant parens around your calls to toString(): I think these
should be dropped.
Repository:
rL LLVM
https://reviews.llvm.org/D27180
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
