[PATCH] Make it easier to use DwarfContext with MCJIT

[Eric Christopher]

On Thu, May 21, 2015 at 5:09 PM David Blaikie <dblaikie at gmail.com> wrote:

> REPOSITORY
>   rL LLVM
>
> ================
> Comment at: include/llvm/DebugInfo/DIContext.h:173
> @@ +172,3 @@
> +  /// The caller is responsible for deallocation once the copy is no
> longer required.
> +  virtual LoadedObjectInfo *clone() const = 0;
> +};
> ----------------
> echristo wrote:
> > loladiro wrote:
> > > echristo wrote:
> > > > loladiro wrote:
> > > > > echristo wrote:
> > > > > > clone is entirely unused.
> > > > > 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.
> > > > 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?
> > > I added this because I need it in Julia. The MCJIT listener interface
> is
> > > ```
> > > NotifyObjectEmitted(const ObjectFile &Object, const
> RuntimeDyld::LoadedObjectInfo &L)
> > > ```
> > > 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.
> > 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.
> This is a bit problematic as the code is entirely untested...
>
> 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.
>
>
Right. Keno said that he was going to swap it to use shared_ptr or some
such shortly...


> Please add some kind of test coverage in-tree or it'd be best to remove
> this API.
>
>
But yes, some sort of in-tree use would be important.

-eric



> http://reviews.llvm.org/D6961
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150522/57d4f5a4/attachment.html>