Michael137 added inline comments.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:225
if (valobj_sp)
+ SyntheticChildrenFrontEnd(*valobj_sp);
Update();
----------------
kastiglione wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > Michael137 wrote:
> > > > this won't initialise the parent constructor though will it? Just
> > > > creates a temporary and immediately destructs it? You might need to put
> > > > it back into the initialiser list and use a ternary
> > > Oh but `SyntheticChildrenFrontEnd` seems to always take a reference. The
> > > interfaces seem to be a little mismatched
> > Perhaps the null-check in the constructor is just redundant and should
> > actually be an assert.
> >
> > @jingham might know more about the assumptions here
> @xgupta reiterating Michael's point, I think this change results in
> mis-construction. Have you run the test suite, I would expect some failures
> with this change.
>
> I agree that this should be an assert. The other option would be to change
> the constructor's signature, using a `lldb::ValueObject &` instead of
> `lldb::ValueObjectSP`, which puts the onus on callers to handle any
> null-ness. Are we aware of any callers passing null?
>
>
FYI, from offline conversation with Jim:
```
This is one of those things where you really should have a general guard
against trying to create a FrontEnd with a null ValueObjectSP, since this isn't
going to do any good. But then you worry "what if one leaks through by
accident" so you put some fallback code when that happens. We were really
insistent that "if we make a programming error in viewing a variable we'd
really like to limit the damage and not crash..." So we didn't go straight to
asserts in all these places the way clang, for instance, does. Taking down a
whole debug session because one variable was bizarre (maybe the DWARF was bad
in a way that confused us...) is not acceptable, there's too much state in the
investigations the debugger supports so we were often over cautious about these
sort of checks.
The better solution would be to go to all the places that produce synthetic
children and make sure that they error out early when told to format empty SP's.
Not sure how doable that is. For most cases, the Synthetic Child Providers
come from type matches, and an empty ValueObjectSP doesn't have a type, so it
would never make a match. But we do have a few "try it against anything"
formatters now, things like the C++ std::function matchers, and the python
functional matchers, so there are places where we might end up screwing this up.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142341/new/
https://reviews.llvm.org/D142341
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
