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