<div dir="ltr">Hi All,<div><br></div><div>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:</div><div><br></div><div><font face="monospace, monospace">MCObjectStreamer::~MCObjectStreamer() {</font></div><div><font face="monospace, monospace">  delete &Assembler->getBackend();</font></div><div><font face="monospace, monospace">  delete &Assembler->getEmitter();</font></div><div><font face="monospace, monospace">  delete &Assembler->getWriter();</font></div><div><font face="monospace, monospace">  delete Assembler;</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>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".</div><div><br></div><div>Target vends MC objects by calling registered ctor functions. E.g.:</div><div><br></div><div><font face="monospace, monospace">MCAsmBackend *createMCAsmBackend(const MCRegisterInfo &MRI,<br></font></div><div><div><font face="monospace, monospace">                                 StringRef TheTriple, StringRef CPU,</font></div><div><font face="monospace, monospace">                                 const MCTargetOptions &Options) </font><span style="font-family:monospace,monospace">const {</span></div><div><font face="monospace, monospace">  if (!MCAsmBackendCtorFn)</font></div><div><font face="monospace, monospace">    return nullptr;</font></div><div><font face="monospace, monospace">  return MCAsmBackendCtorFn(*this, MRI, Triple(TheTriple), CPU, Options);</font></div><div><font face="monospace, monospace">}</font></div></div><div><br></div><div>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).</div><div><br></div><div>There are three approaches I can think of to improve things:</div><div><br></div><div>(1) Keep the raw pointers, but document *very* clearly, recommending the caller stash the result in a unique_ptr immediately.</div><div><br></div><div>(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:</div><div><br></div><div><font face="monospace, monospace">std::unique_ptr<MCAsmBackend> MAB;</font></div><div><font face="monospace, monospace">Target.createMCAsmBackend(&MAB, TheTriple, CPU, Options);</font></div><div><br></div><div><br></div><div>(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:</div><div><br></div><div><font face="monospace, monospace">template <typename T></font></div><div><font face="monospace, monospace">class transient_unique_ptr {</font></div><div><font face="monospace, monospace">public:</font></div><div><font face="monospace, monospace">  transient_unique_ptr(std::unique_ptr<T> InVal) : Val(InVal.release()) {}</font></div><div><font face="monospace, monospace">  transient_unique_ptr(transient_unique_ptr<T> &&Other) : Val(Other.Val) { </font><span style="font-family:monospace,monospace">Other.Val = nullptr; </span><span style="font-family:monospace,monospace">}</span></div><div><font face="monospace, monospace">  transient_unique_ptr&& operator=(transient_unique_ptr<T> &&Other) { </font><span style="font-family:monospace,monospace">std::swap(Val, Other.Val); </span><span style="font-family:monospace,monospace">return *this; }</span></div><div><font face="monospace, monospace">  ~transient_unique_ptr() { assert(!Val && "value should have been handed off"); }</font></div><div><font face="monospace, monospace">  std::unique_ptr<T> take() { auto Tmp = Val; Val = nullptr; return unique_ptr<T>(Val); }</font></div><div><font face="monospace, monospace">private: </font></div><div><font face="monospace, monospace">  T *Val = nullptr;</font></div><div><font face="monospace, monospace">};</font></div><div><br></div><div>This type can also operate without needing a T definition. Usage looks like:</div><div><br></div><div><font face="monospace, monospace">auto MAB = Target.createMCAsmBackend(TheTriple, CPU, Options).take();</font></div><div><br></div><div>Option (1) is a minimum. Option (2) and (3) are both compromises to deal with layering rules, but of those I think (2) probably comes closest to adhering to the principle of least surprise.</div><div><br></div><div>Who has thoughts? If we can settle on an approach I'd love to press forward with the cleanup.</div><div><br></div><div>Cheers,</div><div>Lang.</div></div>