thakis added a comment. Thanks for the fast turnaround!
================ Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:34 + # out/gn/obj/clang/unittests/Format/FormatTests, which seems fine. + output_dir = target_out_dir + deps += [ ---------------- phosek wrote: > What if someone explicitly sets `output_dir` in the invoker? We should either > preserve that value or assert with an error message that overriding > `output_dir` in `unittest`s is unsupported. Good point, done. ================ Comment at: llvm/utils/gn/secondary/llvm/utils/unittest/unittest.gni:35 + output_dir = target_out_dir + deps += [ + "//llvm/utils/unittest/UnitTestMain", ---------------- phosek wrote: > You should check if `deps` is defined and set `deps = []` if not, otherwise > the attempt to add to non-existent list is going to fail. I can't imagine that anyone would want a unit test that doesn't at least depend on the code under test. I agree in principle, but I think in practice this would be dead code 100% of the time. Might as well omit it then :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56116/new/ https://reviews.llvm.org/D56116 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits