kastiglione added inline comments.
================ Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:225 if (valobj_sp) + SyntheticChildrenFrontEnd(*valobj_sp); Update(); ---------------- 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? 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