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

[hintonda]

Summary

Added a new Option template, cl::debug_opt, to the
CommandLine parser that aliases to cl::opt in debug builds, but
decays to a simple class containing only the initialized DataType in
Release builds.

debug_opts are also removed from the parser in release builds, so
they don't take up any additional memory, and are designed to be a
drop-in replacement for cl::opt.

cl::OptionCategory and cl::SubCommand also have debug versions
which disappear in release builds.

[hintonda]

llvm/include/llvm/Support/CommandLine.h

411–412

Templatizing these methods is needed, but could go in its own patch. It seems some were and some weren't, but all are needed to use debug_opt.

llvm/lib/Support/Signals.cpp

42

This was added as an example usage in real code, but doesn't need to be included in this patch if and when lands.

[hintonda]

• Added cl::debug_opt_storage with specializations.

With this change, the current sizes are:

Debug build (note that this doesn't include the size of ArgStr or Desc):

sizeof cl::Option: 136
sizeof cl::opt<bool>: 168
sizeof cl::opt<bool, true> -- with location: 168
sizeof cl::debug_opt<bool>: 168
sizeof cl::debug_opt<bool, true>  with location: 168

Release build (ArgStr and Desc are dropped for cl::debug_opt versions):

sizeof cl::Option: 128
sizeof cl::opt<bool>: 160
sizeof cl::opt<bool, true> -- with location: 160
sizeof cl::debug_opt<bool>: 1
sizeof cl::debug_opt<bool, true>  with location: 8

[beanz]

A very interesting idea. A few thoughts inline.

llvm/include/llvm/Support/CommandLine.h

2019

I suspect that NDEBUG probably isn't the right gating for this. We actually probably want this to be something controlled by a build system option.

llvm/unittests/Support/CommandLineTest.cpp

99

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.

[hintonda]

llvm/unittests/Support/CommandLineTest.cpp

99

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.

llvm/include/llvm/Support/CommandLine.h

 //
 class OptionCategory {
 private:
   StringRef const Name;
   StringRef const Description;
 
   void registerCategory();
 
+protected:
+  OptionCategory() {}
+
 public:
   OptionCategory(StringRef const Name,
                  StringRef const Description = "")
       : Name(Name), Description(Description) {
     registerCategory();
   }
 
   StringRef getName() const { return Name; }

 //
 
 // desc - Modifier to set the description shown in the -help output...
 struct desc {
   StringRef Desc;
 
   desc(StringRef Str) : Desc(Str) {}
 
-  void apply(Option &O) const { O.setDescription(Desc); }
+  template<class Opt>
+  void apply(Opt &O) const { O.setDescription(Desc); }
 };
 
 // value_desc - Modifier to set the value description shown in the -help
 // output...
 struct value_desc {
   StringRef Desc;
 
   value_desc(StringRef Str) : Desc(Str) {}
 
-  void apply(Option &O) const { O.setValueStr(Desc); }
+  template<class Opt>
+  void apply(Opt &O) const { O.setValueStr(Desc); }
 };
 
 // init - Specify a default (initial) value for the command line argument, if
 // the default constructor for the argument type does not give you what you
 // want.  This is only valid on "opt" arguments, not on "list" arguments.
 //
 template <class Ty> struct initializer {
   const Ty &Init;
class ValuesClass {
   SmallVector<OptionEnumValue, 4> Values;
 
 public:
   ValuesClass(std::initializer_list<OptionEnumValue> Options)
       : Values(Options) {}
 
   template <class Opt> void apply(Opt &O) const {
     for (auto Value : Values)
-      O.getParser().addLiteralOption(Value.Name, Value.Value,
-                                     Value.Description);
+      O.addLiteralOption(Value.Name, Value.Value,
   }
 };
 
 /// Helper to build a ValuesClass by forwarding a variable number of arguments
 /// as an initializer list to the ValuesClass constructor.
 template <typename... OptsTy> ValuesClass values(OptsTy... Options) {
   return ValuesClass({Options...});
 }

 };
 template <> struct applicator<StringRef > {
   template <class Opt> static void opt(StringRef Str, Opt &O) {
     O.setArgStr(Str);
   }
 };
 
 template <> struct applicator<NumOccurrencesFlag> {
-  static void opt(NumOccurrencesFlag N, Option &O) {
+  template <class Opt> static void opt(NumOccurrencesFlag N, Opt &O) {
     O.setNumOccurrencesFlag(N);
   }
 };
 
 template <> struct applicator<ValueExpected> {
-  static void opt(ValueExpected VE, Option &O) { O.setValueExpectedFlag(VE); }
+  template <class Opt> static void opt(ValueExpected VE, Opt &O) { O.setValueExpectedFlag(VE); }
 };
 
 template <> struct applicator<OptionHidden> {
-  static void opt(OptionHidden OH, Option &O) { O.setHiddenFlag(OH); }
+  template <class Opt> static void opt(OptionHidden OH, Opt &O) { O.setHiddenFlag(OH); }
 };
 
 template <> struct applicator<FormattingFlags> {
-  static void opt(FormattingFlags FF, Option &O) { O.setFormattingFlag(FF); }
+  template <class Opt> static void opt(FormattingFlags FF, Opt &O) { O.setFormattingFlag(FF); }
 };
 
 template <> struct applicator<MiscFlags> {
-  static void opt(MiscFlags MF, Option &O) {
+  template <class Opt> static void opt(MiscFlags MF, Opt &O) {
     assert((MF != Grouping || O.ArgStr.size() == 1) &&
            "cl::Grouping can only apply to single charater Options.");
     O.setMiscFlag(MF);
   }
 };
 
 // apply method - Apply modifiers to an option in a type safe way.
 template <class Opt, class Mod, class... Mods>
void check_location() const {
     assert(Location && "cl::location(...) not specified for a command "
                        "line option with external storage, "
                        "or cl::init specified before cl::location()!!");
   }
 
 public:
   opt_storage() = default;
 
-  bool setLocation(Option &O, DataType &L) {
+  template<class Opt>
+  bool setLocation(Opt &O, DataType &L) {
     if (Location)
       return O.error("cl::location(x) specified more than once!");
     Location = &L;
     Default = L;
     return false;
   }
 
   template <class T> void setValue(const T &V, bool initial = false) {
void done() {
     Parser.initialize();
   }
 
 public:
   // Command line options should not be copyable
   opt(const opt &) = delete;
   opt &operator=(const opt &) = delete;
 
+  template <class DT>
+  void addLiteralOption(StringRef Name, const DT &V, StringRef HelpStr) {
+    getParser().addLiteralOption(Name, V, HelpStr);
+  }
+
   // setInitialValue - Used by the cl::init modifier...
   void setInitialValue(const DataType &V) { this->setValue(V, true); }
 
   ParserClass &getParser() { return Parser; }
 
   template <class T> DataType &operator=(const T &Val) {
     this->setValue(Val);
     return this->getValue();
void done() {
     Parser.initialize();
   }
 
 public:
   // Command line options should not be copyable
   list(const list &) = delete;
   list &operator=(const list &) = delete;
 
+  template <class DT>
+  void addLiteralOption(StringRef Name, const DT &V, StringRef HelpStr) {
+    getParser().addLiteralOption(Name, V, HelpStr);
+  }
+
   ParserClass &getParser() { return Parser; }
 
   unsigned getPosition(unsigned optnum) const {
     assert(optnum < this->size() && "Invalid option index");
     return Positions[optnum];
   }
 
   void setNumAdditionalVals(unsigned n) { Option::setNumAdditionalVals(n); }

 void ResetAllOptionOccurrences();
 
 /// Reset the command line parser back to its initial state.  This
 /// removes
 /// all options, categories, and subcommands and returns the parser to a state
 /// where no options are supported.
 void ResetCommandLineParser();
 
+#ifndef NDEBUG
+template <class DataType, bool ExternalStorage = false,
+          class ParserClass = parser<DataType>>
+using debug_opt = opt<DataType, ExternalStorage, ParserClass>;
+
+using DebugSubCommand = SubCommand;
+using DebugOptionCategory = OptionCategory;
+
+#else
+class DebugSubCommand : public SubCommand {
+public:
+  DebugSubCommand(StringRef, StringRef = "") {}
+};
+
+class DebugOptionCategory : public OptionCategory {
+public:
+  DebugOptionCategory(StringRef, StringRef = "") {}
+};
+
+template <class DataType, bool ExternalStorage, bool isClass>
+class debug_opt_storage {};
+
+template <class DataType> class debug_opt_storage<DataType, false, false> {
+public:
+  DataType Value;
+
+  // Make sure we initialize the value with the default constructor for the
+  // type.
+  debug_opt_storage() : Value(DataType()) {}
+
+  template <class T> void setValue(const T &V, bool initial = false) {
+    Value = V;
+  }
+  DataType &getValue() { return Value; }
+  DataType getValue() const { return Value; }
+
+  const OptionValue<DataType> getDefault() const { return OptionValue<DataType>(Value); }
+
+  operator DataType() const { return getValue(); }
+
+  // If the datatype is a pointer, support -> on it.
+  DataType operator->() const { return Value; }
+};
+
+// cl::opt_storage: specialized for external location.
+template <class DataType> class debug_opt_storage<DataType, true, false> {
+  void check_location() const {
+    assert(Location && "cl::location(...) not specified for a command "
+                       "line option with external storage, "
+                       "or cl::init specified before cl::location()!!");
+  }
+public:
+  DataType *Location = nullptr; // Where to store the object...
+
+  debug_opt_storage() = default;
+
+  template<class Opt>
+  bool setLocation(Opt &O, DataType &L) {
+    assert(!Location && "cl::location(x) specified more than once!");
+    Location = &L;
+    return false;
+  }
+  template <class T> void setValue(const T &V, bool initial = false) {
+    check_location();
+    *Location = V;
+  }
+  DataType &getValue() { return *Location; }
+  DataType getValue() const { return *Location; }
+
+  const OptionValue<DataType> getDefault() const { return OptionValue<DataType>(*Location); }
+
+  operator DataType() const { return getValue(); }
+
+  // If the datatype is a pointer, support -> on it.
+  DataType operator->() const { return Location; }
+};
+
+// cl::debug_opt: Slimmed down version of cl::opt that doesn't register with the
+// parser or maintain any unnecessary state.
+template <class DataType, bool ExternalStorage = false,
+          class ParserClass = parser<DataType>>
+class debug_opt : public debug_opt_storage<DataType, ExternalStorage,
+                                     std::is_class<DataType>::value> {
+
+public:
+  debug_opt(const debug_opt &) = delete;
+  debug_opt &operator=(const debug_opt &) = delete;
+
+  template <class DT>
+  void addLiteralOption(StringRef Name, const DT &V, StringRef HelpStr) {}
+
+  // setInitialValue - Used by the cl::init modifier...
+  void setInitialValue(const DataType &V) { this->setValue(V, true); }
+
+  // Prints option name followed by message.  Always returns true.
+  bool error(const Twine &, StringRef = StringRef(),
+             raw_ostream & = llvm::errs()) {
+    return true;
+  }
+  //bool error(const Twine &, raw_ostream &) { return true; }
+
+  template <class... Mods> explicit debug_opt(const Mods &... Ms) {
+    apply(this, Ms...);
+  }
+
+  // From Option
+  void setArgStr(StringRef) {}
+  void setDescription(StringRef) {}
+  void setValueStr(StringRef) {}
+  void setNumOccurrencesFlag(enum NumOccurrencesFlag) {}
+  void setValueExpectedFlag(enum ValueExpected) {}
+  void setHiddenFlag(enum OptionHidden) {}
+  void setFormattingFlag(enum FormattingFlags) {}
+  void setMiscFlag(MiscFlags) {}
+  void setPosition(unsigned) {}
+  void addCategory(OptionCategory &C) {}
+  void addSubCommand(SubCommand &) {}
+
+};
+
+#endif
+
 } // end namespace cl
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_COMMANDLINE_H

llvm/lib/Support/Signals.cpp

 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
 using namespace llvm;
 
 // Use explicit storage to avoid accessing cl::opt in a signal handler.
 static bool DisableSymbolicationFlag = false;
-static cl::opt<bool, true>
+static cl::debug_opt<bool, true>
     DisableSymbolication("disable-symbolication",
                          cl::desc("Disable symbolizing crash backtraces."),
                          cl::location(DisableSymbolicationFlag), cl::Hidden);
 
 // Callbacks to run in signal handler must be lock-free because a signal handler
 // could be running as we add new callbacks. We don't add unbounded numbers of
 // callbacks, an array is therefore sufficient.
 struct CallbackAndCookie {

llvm/unittests/Support/CommandLineTest.cpp

StackSubCommand(StringRef Name,
                   StringRef Description = StringRef())
       : SubCommand(Name, Description) {}
 
   StackSubCommand() : SubCommand() {}
 
   ~StackSubCommand() { unregisterSubCommand(); }
 };
 
+TEST(CommandLineTest, DebugOption) {
+  cl::OptionCategory Cat("Some category");
+  bool Flag = true;
+  cl::debug_opt<bool, true> FlagOpt(
+      "flag", cl::desc("Simple flag."), cl::location(Flag), cl::init(false),
+      cl::cat(Cat), cl::DefaultOption, cl::Hidden);
+
+  const char *args[] = {"prog", "--flag"};
+
+  // The value will always be present, and initialized.
+  EXPECT_FALSE(Flag);
+
+#ifndef NDEBUG
+  ASSERT_EQ(FlagOpt.Categories.size(), 1U);
+  ASSERT_EQ(cl::Hidden, FlagOpt.getOptionHiddenFlag());
+  EXPECT_TRUE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
+  EXPECT_TRUE(Flag);
+#else
+  // debug_opt's aren't available as commandline options in release builds.
+  EXPECT_FALSE(
+      cl::ParseCommandLineOptions(2, args, StringRef(), &llvm::nulls()));
+  // Flag remains unchanged.
+  EXPECT_FALSE(Flag);
+#endif
+}
 
 cl::OptionCategory TestCategory("Test Options", "Description");
 TEST(CommandLineTest, ModifyExisitingOption) {
   StackOption<int> TestOption("test-option", cl::desc("old description"));
 
   static const char Description[] = "New description";
   static const char ArgString[] = "new-test-option";
   static const char ValueString[] = "Integer";