[llvm-dev] TargetRegistry and MC object ownership.

[Lang Hames via llvm-dev]

>
> (4) Move target registration out of Support so that it can include the
> required definitions from MC.


Yep -- left that one out. Chalk that up to the fever. This seems like an
ideal solution if we can do it.

-- Lang.

On Wed, Oct 11, 2017 at 11:25 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Lang Hames via llvm-dev <llvm-dev at lists.llvm.org> writes:
>
> > Hi All,
> >
> > While trying to put together an MC-based assembler the other day, I again
> > encountered MC's non-obvious memory ownership rules. The most egregious
> > example of these is MCObjectStreamer's destructor:
> >
> > MCObjectStreamer::~MCObjectStreamer() {
> >   delete &Assembler->getBackend();
> >   delete &Assembler->getEmitter();
> >   delete &Assembler->getWriter();
> >   delete Assembler;
> > }
> >
> > In the depths of a fever from a head-cold, I snapped. I've been hacking
> MC
> > to convert these raw pointers (and worse: references!) to unique_ptrs
> > (apologies to people whose backbends I've broken), but I hit a big
> blocker
> > when I get to Target in "llvm/Support/TargetRegistry.h".
> >
> > Target vends MC objects by calling registered ctor functions. E.g.:
> >
> > MCAsmBackend *createMCAsmBackend(const MCRegisterInfo &MRI,
> >                                  StringRef TheTriple, StringRef CPU,
> >                                  const MCTargetOptions &Options) const {
> >   if (!MCAsmBackendCtorFn)
> >     return nullptr;
> >   return MCAsmBackendCtorFn(*this, MRI, Triple(TheTriple), CPU, Options);
> > }
> >
> > The callee owns the resulting object so ideally we would return a
> > unique_ptr<MCAsmBackend>, but to do this we'd need access to the
> definition
> > of MCAsmBackend which we can't get without violating layering. (We need
> the
> > definition because unique_ptr<MCAsmBackend> can notionally call
> > MCAsmBackend's destructor, though we know it won't here as the whole
> point
> > is to hand ownership back to the caller).
> >
> > There are three approaches I can think of to improve things:
> >
> > (1) Keep the raw pointers, but document *very* clearly, recommending the
> > caller stash the result in a unique_ptr immediately.
> >
> > (2) Have the ctor functions take a unique_ptr<T>* as their first argument
> > and store the result there (thanks to Dave Blaikie for this suggestion).
> > Passing the unique_ptrs by pointer means we don't need a definition for
> T.
> > Usage looks like:
> >
> > std::unique_ptr<MCAsmBackend> MAB;
> > Target.createMCAsmBackend(&MAB, TheTriple, CPU, Options);
> >
> >
> > (3) Introduce a 'transient_unique_ptr<T>' type that never calls T's
> > destructor, and whose purpose is to pass off ownership to a real
> unique_ptr:
> >
> > template <typename T>
> > class transient_unique_ptr {
> > public:
> >   transient_unique_ptr(std::unique_ptr<T> InVal) : Val(InVal.release())
> {}
> >   transient_unique_ptr(transient_unique_ptr<T> &&Other) :
> > Val(Other.Val) { Other.Val
> > = nullptr; }
> >   transient_unique_ptr&& operator=(transient_unique_ptr<T> &&Other) {
> > std::swap(Val,
> > Other.Val); return *this; }
> >   ~transient_unique_ptr() { assert(!Val && "value should have been handed
> > off"); }
> >   std::unique_ptr<T> take() { auto Tmp = Val; Val = nullptr; return
> > unique_ptr<T>(Val); }
> > private:
> >   T *Val = nullptr;
> > };
>
> (4) Move target registration out of Support so that it can include the
> required definitions from MC.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171011/4ab38a1c/attachment.html>