Re: [boost] out_ptr review

sob., 29 cze 2019 o 21:20 JeanHeyd Meneide via Boost napisał(a):

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:

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; 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.

I must admit, I do not understand this argumentation. No destructor to write, but instead a deleter to write. At least destructor is in one place with the constructor. 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.

Of course, if somebody later comes in, and starts adding members and functionality to this class it will get broked. C++ does not prevent users from conciously dong nasty things. But the point of such class would be to only manage a single resource. With this single responsibility in mind no-one will be adding function calls to the class to "improve" it. - 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 != unique_ptr.

Now it looks like you have to define another 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.

If I suddenly change my mind and instead of one resource, I have to manage a list of resources, this is hardly surprising that I will need a new type. I will then have an aggregate, like std::vector<Framebuffer>, and each Framebuffer will be only managing a single resource. I can see no way the problem you describe with leaking resources will sneak in. Doesn't seem more complicated then writing a yet another deleter and using unique_ptr for managing non-pointers. - Bonus Points: boost::noncopyable is no longer needed, just the unique

pointer which already renders the class move-only.

This does not seem a sufficient motivation to switch to out_ptr().

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.

What "scaling" do you mean here? I have seen one example of changing from managing one resource to managing a list of resources, which does not fit into my definition of "scaling". Building classes that have a single responsibility of managing a single resource does address the problem of dealing with exceptions and early returns. Also, I would like to know what you mean by "and more" in this claim. 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.

Do you mean by "scaling" changing for m propagating a single resource to propagting a list of resources? How often do you do that? (Given that you have changed the type and now you have to change every place that uses the code.) A small degree of additional templating Well, "additional templating" doesn't sound as something good. -- if

you have the time and skill for it --

What skills are required here? can also make this example even more

generic and scale it far beyond what you have written in your example.

Again, what do you have in mind whan you say "scale" here? Can you show us an example (and the reason for doing this "scaling")? And

boost::out_ptr would require no additional changes to its usage to adapt.

From what I understand so far, maybe out_ptr would not require additional changes, but the code that uses out_ptr would.

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/FaK1R... . 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.

Again, I do not know what "scaling" do you mean. From what you are saing I gather that a number of projects found it useful, but what does this have to do with "scaling"? The way I was always taught what "to scale" in the context of software developement mean is that when you put N times more resources to your system, the program will run N times more efficient -- as opposed to nonscalable programs, where if you put N times more resources you will not get any (or minimum) performance gain. Regards, &rzej; 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

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost