In D24952 I tried to refactor the Args class to get rid of m_argv. The original motivation for this is that I wanted a way to implement GetArgumentAtIndex() so as to return a StringRef. I could have just returned a StringRef from the m_argv array, but it seems wasteful to calculate the length every time when we were already storing a std::string. Unfortunately the std::string was in a std::list (required so that it would not be invalidated when inserting items into the arg list) so there was no random access. So I wanted to see if I could do better, and that was the motivation for the original patch.
However, after fixing up all the places in the code to use the "new" Args, I wasn't completely satisfied. I like that the old code could just give you an argv pointer that you can pass to a system API. So I started over and ended up with this. Quick overview of changes:
- Instead of using a std::list<std::string> to own the memory, we use a std::vector<std::unique_ptr<char[]>>. This provides random access, which is something list couldn't provide, and it also provides non-const access to the underlying buffer, which is something that std::string couldn't provide, leading to a lot of const_cast which are no longer necessary.
- Each entry's quote char, backing memory (i.e. std::unique_ptr<char[]>), and StringRef with precomputed length are stored together in a single entry. This guarantees by design that the various arrays' lengths do not need to stay in sync with each other, because there is only one array.
- Some longstanding undefined behavior is documented and left for someone else to fix.
- Asserts are added to the code to document the pre-conditions. I know we are allergic to asserts, but I want to emphasize that none of these asserts have anything to do with the input parameters. The asserts are ALWAYS on internal class state.
With this change it should finally be easy to change GetArgumentAtIndex() to return a StringRef, because it can just return m_entries[idx].ref.