[Lldb-commits] [PATCH] D114791: [lldb] Clarify StructuredDataImpl ownership

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM with some minor nits.



================
Comment at: lldb/bindings/lua/lua-wrapper.swig:31
    lldb::BreakpointLocationSP bp_loc_sp,
    StructuredDataImpl *extra_args_impl
 )
----------------
May be we should keep consistency with the `python-wrapper.swig` definition ?


================
Comment at: lldb/source/API/SBThreadPlan.cpp:72-73
   if (thread)
-    m_opaque_wp =
-        std::make_shared<ThreadPlanPython>(*thread, class_name, nullptr);
+    m_opaque_wp = std::make_shared<ThreadPlanPython>(*thread, class_name,
+                                                     StructuredDataImpl());
 }
----------------
labath wrote:
> I haven't been able to figure out how/if this works. As far as I can tell, this object will get destroyed immediately after construction due to lack of any shared_ptrs pointing to it.
I agree in this case, m_opaque should be either a UP or SP but not a week_ptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114791