[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think Davide's point is that it's not clear what motivated this change and why this doesn't have a regression test. I'm also a bit confused by the description ("We saw a crash recently that looks related to we had good ValueObjectSP for some Cocoa summary providers."). If there isn't a way to test it, then maybe a sentence about why and some background information would be helpful (backtrace and radar numbers are usually fine).

Anyway, from what I can understand in the context is that we saw crashes that looks like we accessed a ValueObject nullptr from these function. And I think this change itself is fine as that's apparently how `GetSyntheticChildAtOffset` communicates errors.

I'm just a bit lost how we can get into this code path and I assume that's also the reason why there is no test. `valobj` appears to be valid, otherwise we wouldn't have gotten so far into this function. So `GetSyntheticChildAtOffset` returns a nullptr in this situation only if either the CompilerType we pass in is invalid or we can't complete the decl behind it. But the type we pass in is `BasicTypeObjCID` and I can't see a situation where we could end up not figuring out the size of plain `id` (the decl behind that is to my knowledge a builtin decl).


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

https://reviews.llvm.org/D84272