[Lldb-commits] [lldb] [lldb] Tighten validity check in ExtractIndexFromString (PR #229930)

[Alex Langford via lldb-commits]

bulbazord wrote:

> If I'm following the code correctly, this will end up doing some parsing based on how a user spelled the name of a path component. So this change means that we won't allow users to type `foo[7 ]`. That seems a silly thing to do, but if you are cutting and pasting the index from somewhere it might inadvertently pick up a space and then to understand the error "no variable named foo[7 ]" when you KNOW there's an item 7 in the array you have to notice the space, which can be easy to miss.
> 
> So I think we should preserve space stripping when parsing these values. If this was just internal, we could expect OURSELVES not to do this, but I don't think we should impose that on users.
> 
> We never explicitly said you can't use negative index values, but this particular routine was returning a size_t which is an unsigned so it was never going to do the right thing for negative index values. So not supporting them at all seems strictly better. And if you did want to produce a synthetic child that looks like an array with an index set that had positive and negative values, you'd do that by implementing your own get_child_by_name and that would bypass this convenience routine. So I am fine with dropping support for +/-.

I'm ok with ignoring whitespace between brackets. I would also be ok with re-adding support for +/- in a follow-up change so one could implement python-like semantics for negative indices of list-like structures. But that would be a more involved change and I really just want to use StringRef here for a different goal altogether...

Will update with some whitespace trimming logic.

https://github.com/llvm/llvm-project/pull/229930