Re: out_ptr review

From: JeanHeyd Meneide (phdofthehouse_at_[hidden])
Date: 2019-06-29 19:19:45

Dear Phil Endecott,

     Thank you for your review.

     There has been a consistent view that there should be beautiful,
hand-written wrappers around all of the C functions that you or your
application care about. I am convinced these wrappers are both
fundamentally flawed and wrong for users.

     Let's look at your Frambuffers example:

class Framebuffer: boost::noncopyable
{
  GLuint handle;
public:
  Framebuffer() { glGenFramebuffers(1, &handle); }
  ~Framebuffer() { glDeleteFramebuffers(1, &handle); }
};

To start, boost::out_ptr::out_ptr accomodates non-pointer handles, the same
as std::unique_ptr accommodates them. Your code can be rewritten using a
std::unique_ptr<GLuint, frame_buffer_deleter>:

struct gl_handle {
     GLuint handle;

     friend bool operator ==(GLuint, nullptr);
     friend bool operator ==(nullptr, GLuint);
     friend bool operator ==(GLuint, GLuint);
     friend bool operator !=(GLuint, nullptr);
     friend bool operator !=(nullptr, GLuint);
     friend bool operator !=(GLuint, GLuint);
};
struct frame_buffer_deleter {
     using pointer =  gl_handle;

     void operator( gl_handle handle){
          glDeleteFramebuffers(handle.handle, 1);
     }
};

class Framebuffer
{
  std::unique_ptr<GLuint, frame_buffer_delete>;
public:
  Framebuffer() { glGenFramebuffers(1,
boost::out_ptr::out_ptr<GLuint>(handle)); }
};

"That's so much more code, why would anyone do that?!" you ask. For the
following reasons:

   - There's no more destructor to write. If this class ever got marginally
   more complicated than just be a specifically-named unique_ptr, allocation
   of the framebuffer may succeed but an exception can be thrown, locking
   driver resources because you never deleted them:
   http://coliru.stacked-crooked.com/a/910aa40650884efb and
   http://coliru.stacked-crooked.com/a/48126c474d033c5d both demonstrate
   that. There is a reason why the rule of zero -- by R. Martinho Fernandes
   and included in Scott Meyers's book and spoken about around the world --
   became so popular and important in C++11.
   - Reusable: right now I have programmed to specifically write to your
   framebuffer example. The code I wrote to do this in reality (and ported to
   multiple platforms, including for Vulkan and DirectX) used unique_ptr to
   hold the handle and I could make it handle creating more than 1 framebuffer
   by adding a sized deleter. Note that such a deleter makes it a strong type:
   unique_ptr<GLuint[], sized_deleter> != unique_ptr<GLuint, regular_deleter>.
   - Your code hides it in the internals of Framebuffer and hard-codes it:
   if you were to ever change the class, you would need to change the ctor,
   dtor, and anywhere else that the assumption takes place, or write an
   entirely new class "Framebuffer*s*". If written in the same style of
   your other one, it would be vulnerable to the same problems.
   - Bonus Points: boost::noncopyable is no longer needed, just the unique
   pointer which already renders the class move-only.

There are similar problems with your PNG example.

     All of this goes to show that while you can write cute one-off
examples that seem simpler, they do not scale to the fundamental reality
that is handing resources, dealing with exceptions, and more in C++ APIs.
out_ptr scales far beyond what you just wrote, handles the fact that
someone can change their pointer type from `handle` to `handle[]` for
multiple resources, and more. A small degree of additional templating -- if
you have the time and skill for it -- can also make this example even more
generic and scale it far beyond what you have written in your example. And
boost::out_ptr would require no additional changes to its usage to adapt.

     The rest of your review comments have already been addressed in the
feature/boost.review branch and have been answered in other out_ptr
threads. This API has been used with libavformat, Python's C API, OpenGL,
DirectX, COM interfaces, and more. Previous iterations of this class have
been used in VMWare driver infrastructure, all of Microsoft WRL library (as
an improvement over its original CComPtr), and more (see the proposal p1132
<https://thephd.github.io/vendor/future_cxx/papers/d1132.html#motivation>,
which details previous iterations of this). It has also been written by
Raymond Chen, in his pursuit for a cleaner interface, but with some issues
regarding destructor order for temporaries in function calls in C++ (which
were correctly called out in the paper and the documentation before he
wrote the article). See:
https://devblogs.microsoft.com/oldnewthing/20190429-00/?p=102456

     Finally, I have been vending this on my own time at no cost to
individuals and companies for over a year, starting from its earlier
versions from a (now superseded) repository:
https://groups.google.com/a/isocpp.org/d/msg/std-proposals/8MQhnL9rXBI/FaK1RZNnCwAJ.
I was also not the one to make this idea in the first place: I was taught
the technique  over 4 years ago by Mark Boyall in a C++ chat room.

     While I appreciate your criticisms, I do not think your examples and
suggestions scale to Fortune 500 company code base infrastructure or small
individual hobby codebases. out_ptr, however, does and has, and should be
included in the Standard. And maybe Boost, too.

     Thank you for your time.

Sincerely,
JeanHeyd Meneide