labath added a comment.

In D66345#1634800 <https://reviews.llvm.org/D66345#1634800>, @teemperor wrote:

> In D66345#1633172 <https://reviews.llvm.org/D66345#1633172>, @labath wrote:
>
> > In D66345#1633118 <https://reviews.llvm.org/D66345#1633118>, @teemperor 
> > wrote:
> >
> > > Not sure if we can get rid of StringList so easily as we still have 
> > > SBStringList.
> >
> >
> > We can keep the SBStringList. We can just have it be backed by a 
> > `vector<string>` instead of the StringList thingy...
>
>
> 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 :).


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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66345/new/

https://reviews.llvm.org/D66345



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to