<div dir="ltr"><br><br><div class="gmail_quote">On Thu, May 21, 2015 at 5:09 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">REPOSITORY<br>
  rL LLVM<br>
<br>
================<br>
Comment at: include/llvm/DebugInfo/DIContext.h:173<br>
@@ +172,3 @@<br>
+  /// The caller is responsible for deallocation once the copy is no longer required.<br>
+  virtual LoadedObjectInfo *clone() const = 0;<br>
+};<br>
----------------<br>
echristo wrote:<br>
> loladiro wrote:<br>
> > echristo wrote:<br>
> > > loladiro wrote:<br>
> > > > echristo wrote:<br>
> > > > > clone is entirely unused.<br>
> > > > It's supposed to be available for the client to be able to obtain a copy of the LoadedObjectInfo, since the one it gets from MCJIT is passed by reference. We could probably change that one to a shared_ptr, but I didn't want to put that API breakage into the same commit.<br>
> > > I guess, it might be nicer to make this clear. I'm not a fan of requiring people to clone it etc. Maybe I'm just not seeing where it's going to be used off the top of my head, and can be added in the future if we have a user of the functionality?<br>
> > I added this because I need it in Julia. The MCJIT listener interface is<br>
> > ```<br>
> > NotifyObjectEmitted(const ObjectFile &Object, const RuntimeDyld::LoadedObjectInfo &L)<br>
> > ```<br>
> > but I need to store the LoadedObjectInfo for later (to query the DWARFContext), so I added the clone method as a workaround. Better alternatives would be most appreciated of course.<br>
> Not my favorite set of interfaces, no, but I don't have anything else in mind at the moment and hopefully someone else can come up with something better.<br>
This is a bit problematic as the code is entirely untested...<br>
<br>
For example - I've fixed the API to use unique_ptr, which may break your usage & I have little idea if my change is correct.<br>
<br></blockquote><div><br></div><div>Right. Keno said that he was going to swap it to use shared_ptr or some such shortly...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Please add some kind of test coverage in-tree or it'd be best to remove this API.<br>
<br></blockquote><div><br></div><div>But yes, some sort of in-tree use would be important.</div><div><br></div><div>-eric</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D6961&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ZfiWzXMXlOn5GXyRBAEcklzcz7tjxNtSVZJplzTNQes&s=WKmXwa0fU7Zpe8rn7Z538O6DX7G-QMviTGMD2kZbZNU&e=" target="_blank">http://reviews.llvm.org/D6961</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ZfiWzXMXlOn5GXyRBAEcklzcz7tjxNtSVZJplzTNQes&s=6zg6brlN4GNQA0jK-MMtdsRtf-0thOwlywnHy8feLIo&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</blockquote></div></div>