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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
