Added support for for-range iterating over StringList. There is no reverse-iterator support because it seems
no one is doing that in LLDB. Also added some tests and migrated LLDB code over to for-range loops
Definitely an improvement, though one day, i'd like to get rid of the StringList class (and various other XXXList classes) completely :)
maybe EXPECT_THAT(recorded, testing::ElementsAre("a", "b", "c")) ?
const size_t max_len = std::max_element(commands_found.begin(), commands_found.end());
Constructing the const_type_name isn't necessary anymore. Just change the if to:
m_strings.reserve(m_strings.size() + strings.GetSize());
What does "ForRangeSingle" mean here? The name led me to believe you were going to test a 1-element StringList, but I see three elements.
I rarely touch the SB* classes, but I always thought we just want them as thin wrappers around LLDB classes? If not, then yeah, removing StringList seems good to me :).
Don't think that works without a lambda that calls .size(), but StringList actually has a method for finding the max string length so we might as well use that.
Yeah, that was originally just for one element (before I just made one test with three elements which should be enough coverage).
Yes, the SB classes should be thin, but I would interpret the "thinness" as "they shouldn't implement non-trivial logic", not as "there must be a non-SB class of the same name which they delegate to". I don't think the latter would be fair because the SB classes are basically frozen solid, and the second requirement would inflict that solidity on the rest of lldb too. We should have the ability refactor, clean up or otherwise improve internal lldb interfaces even if that means diverging the SB and non-SB class variants. Now, that divergence has a cost, and that should be weighed against the other benefits of the change, but it shouldn't be an immediate show-stopper. In this case, I would say that the cost of that is quite low, because StringList essentially is a std::vector<std::string> and anything extra it provides on top of that (e.g. LongestCommonPrefix) can be easily implemented as a free function.
The other thing to think about is the SB layer must be thread safe. StringList is not thread safe right now, but it probably should be. We might need to keep the internal version of StringList and make sure it is thread safe. That will ensure that our buildbots always run clean.