[Lldb-commits] [PATCH] D26883: Demonstrate proposed new Args API

[Zachary Turner via lldb-commits]

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

The purpose of this patch is to demonstrate my proposed new API to the `Args` class.  There are a couple of things I'm trying to demonstrate here:

1. range-based for loops by iterating directly over the `Args` instance itself.  This should be the preferred way to iterate over arguments and should always be used unless index-based iteration is required.

2. range-based for loops by iterating over `args.entries()`.  This should be used when you only want to iterate a subset of the arguments, like if you want to skip the first one (e.g. `for (const auto &entry : args.entries().drop_front())`.  This prevents many types of bugs, such as those posted to the list recently in which we repeatedly access `args[0]` instead of `args[i]`.

3. Index-based iteration through the use of `operator[]`.  Should only be used when you need to do look ahead or look-behind, or a command takes positional arguments.  Prefer range-based for otherwise.  This method supersedes `GetArgumentAtIndex` and the long term plan is to delete `GetArgumentAtIndex` after all call-sites have been converted.

4. Direct access to `StringRef` through the use of `entry.ref`.  Should always be used unless you're printing or passing to a C-api.

5. Direct access to the quote char through the use of `entry.quote`.  Supersedes `GetArgumentQuoteCharAtIndex`.

6. Access to a null-terminated c-string.  Should only be used when printf'ing the value or passing to a C-api (which, btw, should almost never be done for any kind of string manipulation operation, as `StringRef` provides everything you need).  Long term I plan to delete this accessor, the only thing blocking this is the heavy use of printf-style formatting (which I have a solution for in mind).  In any case, it should not be relied on for any kind of algorithmic access to the underlying string.

7. A couple of simple examples of how this proposed new API can trivially reduce string copies and improve efficiency.

All dependent calls have already been updated to take `StringRef`, so the "phase 2", so to speak, will be to begin eliminating calls to `GetArgumentAtIndex` using the methods described above.  Once they are all gone, the plan is to delete `GetArgumentAtIndex` and `GetQuoteCharAtIndex`.

In the more distant future, if I can make my `Printf` alternative a reality, I plan to delete c-style string access entirely by removing the `c_str()` method from `ArgEntry`.


https://reviews.llvm.org/D26883

Files:
  include/lldb/Interpreter/Args.h
  source/Breakpoint/BreakpointIDList.cpp
  source/Commands/CommandObjectCommands.cpp
  source/Commands/CommandObjectFrame.cpp
  source/Commands/CommandObjectProcess.cpp
  source/Commands/CommandObjectSettings.cpp
  source/Interpreter/CommandInterpreter.cpp
  source/Interpreter/OptionValueDictionary.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26883.78617.patch
Type: text/x-patch
Size: 13041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161119/f1606c1a/attachment.bin>