[PATCH] D62264: [CommandLine] WIP: Add new debug_opt that reduces down to size of DataType in Release builds.

Wed May 22 15:41:47 PDT 2019

hintonda marked an inline comment as done.
hintonda added inline comments.

================
Comment at: llvm/unittests/Support/CommandLineTest.cpp:99
+#else
+  // debug_opt's aren't available as commandline options in release builds.
+  EXPECT_FALSE(
----------------
beanz wrote:
> I'm a little concerned about how this would work. For one thing it makes testing and reproducing bugs in release builds trickier. I suspect with your patch as-is if you converted many of the target options to `debug_opt` `check-llvm` would fail gloriously. I'm not sure that is the behavior we want because I know many distribution workflows rely on running the tests against the build before packaging the distribution.
This version gets rid of as much as possible -- sort of a benchmark.  

Depending on what we want/need for release tests, I could see memory saving alternatives that would allow the tests to run as is.  However, I'd imagine there are still plenty of cases where this approach would work out fine.

For example, there are already cases where Options are completely #ifdef'd away, e.g., `--debug` and friends.  The problem with that approach compared with this one is that it requires you litter the code with #ifdef's.  Assuming we ultimately get the savings my tests indicate, this approach would help keep the code clean at the cost of a very small amount of memory.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62264/new/

https://reviews.llvm.org/D62264