[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: clayborg, labath.
zturner added a subscriber: lldb-commits.

The major blocker to this until now has been that we can't have global constructors, no matter how trivial.

Recently LLVM obtained the `StringLiteral` class which addresses this.  This class has a `constexpr` constructor, so as long as a global `StringRef` (or class/struct containing a `StringRef` is declared `constexpr`, the `StringRef` can be initialized with a `StringLiteral` and not invoke a global constructor.  This probably sounds confusing, but in practice it's simple.  It looks like this:

  // ok
  constexpr StringRef S = llvm::StringLiteral("foo");
  
  // ok
  constexpr StringLiteral T("foo");
  
  // not ok, global constructor invoked.
  StringLiteral U("foo");
  
  struct Foo {
    constexpr Foo(StringRef S) : S(S) {}
    StringRef S;
  };
  
  // ok
  constexpr Foo F(llvm::StringLiteral("foo"));

Admittedly, it's pretty verbose to type out `StringLiteral` everywhere.  To get around this, I alias it to `L` in the individual TUs where we use it.  This way we can write:

  using L = llvm::StringLiteral;
  static constexpr OptionDefinition[] g_defs = { ..., ..., L("opt"), ..., ..., L("help text") };

Again, note that no global constructor is invoked here, even though this is an array.

The advantage of doing all of this is that we can now have access to all the wonderful member functions of `StringRef`.  Upon fixing up all the compilation failures (there were very few however) triggered by the change, we can already see some code being simplified, and in the future I would like to extend this to other classes like `RegisterInfo` etc as well.

Note that this patch is incomplete as is.  Before submitting, I would need to go change EVERY global definition of `OptionDefinition` to be marked `constexpr`, otherwise a global constructor would be invoked.  I intentionally did it in only 2 places here, just to illustrate the idea, before changing the code everywhere.


https://reviews.llvm.org/D27780

Files:
  lldb/include/lldb/Interpreter/Options.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Host/common/OptionParser.cpp
  lldb/source/Interpreter/Args.cpp
  lldb/source/Interpreter/OptionGroupOutputFile.cpp
  lldb/source/Interpreter/Options.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27780.81487.patch
Type: text/x-patch
Size: 14795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161214/48a2f940/attachment-0001.bin>