EricWF marked 5 inline comments as done. EricWF added inline comments.
================ Comment at: lib/Driver/ToolChains/Clang.cpp:3269-3274 + const bool IsAssertBuild = #ifdef NDEBUG - CmdArgs.push_back("-disable-llvm-verifier"); - // Discard LLVM value names in -asserts builds. - CmdArgs.push_back("-discard-value-names"); + false; +#else + true; #endif ---------------- bogner wrote: > It might be a few more characters, but I feel like this is more readable if > you put entire statements in the branches of the #if, ie: > > #ifdef NDEBUG > const bool IsAssertBuild = false; > #else > const bool IsAssertBuild = true; > #endif Ack. Done. ================ Comment at: test/Driver/clang_f_opts.c:522 +// RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-DISCARD-NAMES %s +// RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-NO-DISCARD-NAMES %s +// CHECK-DISCARD-NAMES: "-discard-value-names" ---------------- lebedev.ri wrote: > I wonder if it is also possible to check that if neither of > `-f[no-]discard-value-names` is specified, what happens. > The caveat is of course the asserts-vs-no-asserts build type. I don't think so, at least not easily and without changes to the `lit` configuration. It's gone untested this long, I would love to get this patch in without being responsible for adding those tests. https://reviews.llvm.org/D42887 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits