[Lldb-commits] [lldb] [LLDB] Add array subscription and integer parsing to DIL (PR #138551)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

I'm sorry, I put this off because it wasn't clear how to respond immediately -- and then it dropped off my radar.

I have a bunch of comments, but overall, I think this could work.

My main objection is to the use of the APInt class. If you want to use it (which I'm fine with, but I also think a plain (u)int64_t would work for this purpose as well), then I think you should use it properly: using the operations defined on the class itself (e.g. `slt`/`ult`) -- instead of converting it to an plain integer first chance you get. As it stands now your parser will happily accept an incredibly long integer, but then you'll assert when `getZExtValue` cannot convert that to an uint64_t. There's also some confusion about whether the value is signed or unsigned. You can't just switch between `getZExtValue` and `getSExtValue` and expect the result to make sense (hint: `getSExtValue` of "0x80" parsed into an APInt is -128)

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