[PATCH] D137487: [clang][Interp] Start implementing builtin functions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/Interp.h:1357
+    if (InterpretBuiltin(S, PC, BID)) {
+      NewFrame.release();
+      return true;
----------------
erichkeane wrote:
> tbaeder wrote:
> > erichkeane wrote:
> > > What is going on here?  Why is this not a leak?  
> > The current frame is `delete`d when successfully returning the value of the function, this happens in the `Ret` (and `RetVoid`) function in `Interp.cpp`.
> > 
> > No opinion on whether that's the best way to do it though.
> I really dont like doing it this way, I see we're doing it the same in 1366, but if we're expecting someone else to delete a unique-ptr, we should give them ownership of the whole unique-ptr.  This mechanism is just obscuring the ownership semantics that unique_ptr is supposed to imply.
I blame @aaron.ballman :)

I'm gonna write it on my list and create a follow-up patch.


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

https://reviews.llvm.org/D137487