std::unique_ptr<void, decltype(&CloseHandle)> p(CreateFile(...), &CloseHandle);
(side question:
can I specialize default_delete for MyType*?)
Then again, I don't think mirroring default_delete is a good idea,
since I at least
have no clue what this mystical default_nonptr_delete would do. :)
Perhaps defaulting
the deleter type to a function pointer would be a better solution
after all, that doesn't
prevent using functors where necessary.
#include <memory>
int open() { return 1; }
void close(int) {}
template<typename T, typename deleter, deleter d, T null_value = T {}> struct value_deleter {
struct wrapper {
T value;
wrapper(T t) : value(t) {}
bool operator==(std::nullptr_t) { return value == null_value; }
bool operator!=(std::nullptr_t) { return value != null_value; }
T operator*() { return value; }
wrapper(std::nullptr_t) : value(null_value) {}
wrapper() : value(null_value) {}
};
typedef wrapper pointer;
void operator()(wrapper w) {
if (w.value != null_value)
d(w.value);
}
};
int main() {
std::unique_ptr<int, value_deleter<int, decltype(&close), &close>> ptr(open());
}
template<typename T, typename CloseType>
using unique_handle<T> = std::unique_ptr<T, value_delete<T, CloseType>>
No, but you are asking the Standard library to duplicate a bunch of code and functionality, and expend a lot of valuable Committee time, when the existing abstraction can handle the problem just fine. Adding things to the Standard is a lot more expensive than the quick wrapper I belted out in five minutes. It takes a whole lot more than that. Every second the Committee spends debating this class, they could be spending on concepts, or modules, or something like that, which would deliver vastly more benefit. The time of the Committee is far more valuable than the few minutes it takes to use this particular abstraction.
I didn't mean to sound like a jerk, and I didn't mean to jeer at you. Silly Google Groups ate my original reply, so let me be brief.Your need for very tight binary code is just that, your need. The Standard needs to consider everyone's needs. Your need is very niche, and very specialist. There's no reason the Committee would severely impair a general-purpose interface which works fine for virtually everybody for the vast minority. It's much better to ask you to write a new class for your specific need than to ask everybody to write a new class. They already considered the impact of unique_ptr's interface, and decided on it. It shouldn't yield any worse code size than your RAIIWrapper, and if it did, then call your compiler vendor- there's nothing the Committee can do about poor QoI.As for non-pointer resources, I believe it should be perfectly possible to use unique_ptr with them. If not, consider proposing an extension to it that would change this. Introducing a second class for the same purpose- unique ownership of a resource- is not the solution. After all, what next, another new class for shared ownership?As for RAIIFunction, you should at least start by converting it's interface to mirror that of the Standard, and propose an interface for it which matches that of the normal Standard components, like std::scope_exit or something.
I've never really understood the point ofa generalized scope_exit syntax in C++. It makes more sense in a language like C, where RAII objects don't exist. I mean, if you want to create something and ensure it gets cleaned up, then write an object for it. It's that simple. That way, when you use it in multiple places, you won't have to copy all those silly "scope exist" bits around. You can also transfer the cleanup, thanks to move.
I wrote one for the xml_document type in LibXML2. It even automatically converts itself into an xml_document*, so you can use it anywhere you could before. It just cleans up after itself.
In virtually all of these "scope exit" cases, there is a specific resource that you want to key off of and clean up. That's what RAII is designed for, so just write a little RAII wrapper and go to it.
If you want to bring the scope stuff from D into C++, then at least bring in the useful part. Namely, being able to differentiate between scope exiting due to failure (ie: the stack unwinding from throwing an exception) and scope exiting due to success (ie: the stack ending naturally, without exception unwinding). Oh, and make it a real language feature, not some hack function bit that you have to use a non-capturing lambda to access.
namespace std
{
template<typename T> class scope_exit_impl
{
T m_tFunc;
bool m_bExecute;
public:
scope_exit_impl(T&& arg) : m_tFunc(std::move(arg)), m_bExecute(true)
{
}
scope_exit_impl(const T& arg) : m_tFunc(arg), m_bExecute(true)
{
}
~scope_exit_impl()
{
if(m_bExecute)
{
m_tFunc();
}
}
//
// Cancel
scope_exit_impl& cancel()
{
m_bExecute = false;
return *this;
}
//
// Invoke...(e.g. early invocation)
scope_exit_impl& invoke(bool bCancelAfterInvoke = true)
{
m_tFunc();
m_bExecute = !bCancelAfterInvoke;
return *this;
}
};
template<typename T> scope_exit_impl<T> scope_exit(T t)
{
return scope_exit_impl<T>(std::move(t));
}
}
void Test()
{
auto&& exit_var = std::scope_exit([]()
{
printf("Exiting Test\n");
});
printf("In Test\n");
}
void Test2()
{
auto exit_var = std::scope_exit([]()
{
printf("Exiting Test 2\n");
});
printf("In Test 2\n");
}
void Test3()
{
auto &&fn = std::bind(&MessageBoxA, static_cast<HWND>(0), "Test 3", "Test 3", MB_TOPMOST);
auto &&exit_var = std::scope_exit(fn);
printf("In Test 3\n");
}
void Test4()
{
HANDLE hEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
auto exit_var = std::scope_exit([&hEvent]()
{
if(nullptr != hEvent)
{
CloseHandle(hEvent);
}
});
printf("In Test 4\n");
}
void TestCancel()
{
auto exit_var = std::scope_exit([]()
{
printf("Exiting Cancel\n");
});
printf("In TestCancel\n");
exit_var.cancel();
}
void TestConst()
{
const auto &&exit_var = std::scope_exit([]()
{
printf("Exiting Const\n");
});
// exit_var.invoke(); // compiler error as expected
printf("In TestConst\n");
}
int _tmain(int argc, _TCHAR* argv[])
{
Test();
Test2();
Test3();
Test4();
TestCancel();
TestConst();
return 0;
}
> Can we agree that such a thing would be a good library to propose adding to
> the standard library?
I think it would be a worthwhile addition.
> Can we agree that such a thing would be a good library to propose adding to
> the standard library?
I think it would be a worthwhile addition.
If anyone is to write a proposal, I suggest to contact Andrei Alexandrescu because he and Petru Marginean wrote a similar feature, ScopeGuard, back in 2000 [1]. More recently Andrei did a revamp of ScopeGuard using C++11 features and presented it in the C++ and Beyond 2012. An implementation is provided in Folly [2]. See also the references in [2].
HTH,
Cassio.
I don't understand this thinking. I don't see how this throws out "generic programing or functional programming." The whole point of a multi-paradigm language is that you get to use the paradigm that best serves your needs at the time. Nobody's stopping you from putting this in generic or functional code.
Exiting scope is best done as a RAII because virtually every cases of it is bound to some specific resources. Once it's in a RAII object, you can actually pass it around, transfer ownership, and all of those other things you can't do with a static scope exit.
On Tuesday, November 6, 2012 11:10:23 AM UTC-6, Nicol Bolas wrote:
I don't understand this thinking. I don't see how this throws out "generic programing or functional programming." The whole point of a multi-paradigm language is that you get to use the paradigm that best serves your needs at the time. Nobody's stopping you from putting this in generic or functional code.
Exiting scope is best done as a RAII because virtually every cases of it is bound to some specific resources. Once it's in a RAII object, you can actually pass it around, transfer ownership, and all of those other things you can't do with a static scope exit.
I like the idea of having generic RAII wrappers. I don't like the idea of writing new classes to encapsulate every type of resource.
FILE *pFile = fopen(...);
RAIIExitScope SomeNameIDontWantToConflictWithAnything([&] {fclose(pFile);})
FILE *pFile = fopen(...);
scope(exit) { fclose(pFile); }
MyFile file(...);
MyFile SomeFunction(...)
{
MyFile file(...);
//Do stuff with file.
return file;
}
The generic wrappers for me extend generic programming and functional programming. They help produce cleaner code, more quickly. Writing object wrappers around everything is tedious and produces a lot of code that will never get used, because when you work -- as I and my colleagues do, at a very low level, you need to see which system APIs are being used, and an object wrapper obscures that, where as the classes I've suggested here do not.
I don't think that is limited to guys that spend a lot of time at the level I'm speaking of (I do a lot of code injection, API hooking, etc.) -- I think it applies generally. In the Windows world, WTL was a hit because MFC obscured and bloated too much.
There is definitely a place for object wrappers. I'm sure the new thread classes, and file system objects will have huge benefit. But, despite having had IOStreams for a very long time, there is still a very legitimate need to use open/close/read/readv/pread/write/writev/pwrite or their system-level equivalents on other platforms!
But they won't. History has made it very clear that people will pick between them based on what is easiest at the time. And it is far easier to use a scope(exit) than it is to make a proper object, even if you stick that in 30 places in your source code.
We shouldn't add features that make writing bad code easier; C++ has plenty of that as is. RAII is the standard C++ method for solving resource cleanup issues; we shouldn't make it easy to avoid RAII in favor of something that looks like RAII but is easier to write in the short-term and is a huge burden to maintain in the long-term.
I guarantee you that the very minute scope(exit) or whatever hits the language/library and a functioning compiler implementation, you'll see far fewer RAII objects and a lot more mysterious resource failures.
But they won't. History has made it very clear that people will pick between them based on what is easiest at the time. And it is far easier to use a scope(exit) than it is to make a proper object, even if you stick that in 30 places in your source code.
We shouldn't add features that make writing bad code easier; C++ has plenty of that as is. RAII is the standard C++ method for solving resource cleanup issues; we shouldn't make it easy to avoid RAII in favor of something that looks like RAII but is easier to write in the short-term and is a huge burden to maintain in the long-term.
I don't think so. Because there is a big advantage to putting things like an RAIIWrapper<RAIIDeleter(close), -1> m_iFileDescriptor; right into your File object, so that you don't need an explicit destructor.
The functionality of the code will be improved, in that you're more likely to get resources released and better exception behavior. But it's going to be that much harder to eventually get them to use actual RAII, because they'll just say, "Oh, I'll just use a scope(exit); no point in creating an object. Everyone knows those are all slow and bloated, after all."
It reinforces the already existing (and incorrect) stigma that object == bad, repeated local code == good.
It reinforces the already existing (and incorrect) stigma that object == bad, repeated local code == good.
Local code has advantages; we should be finding ways to embrace those advantages. Lambdas, for instance, are a good first step.
Do you believe the status quo is good? Because if we do nothing, that is where we will stay.
Writing objects in C++ is incredibly tedious. I'd like to reduce that tedium; otherwise, for "small" things, people just fall back on abusing native types and writing C style code.
Which people? Your coworkers, perhaps. Other people, perhaps. But there are many C++ programmers who embraced RAII long ago, regardless of how "incredibly tedious" it was.
I personally would find it far more tedious to write little lambdas and such in every place I use a resource just to prevent resource leaks, when writing a single object once would solve the problem everywhere.
On 7 November 2012 12:38, Nicol Bolas <jmck...@gmail.com> wrote:
Which people? Your coworkers, perhaps. Other people, perhaps. But there are many C++ programmers who embraced RAII long ago, regardless of how "incredibly tedious" it was.
And removing the tedium of writing small classes will help us too.I personally would find it far more tedious to write little lambdas and such in every place I use a resource just to prevent resource leaks, when writing a single object once would solve the problem everywhere.
For instance, do you really find:
auto i = std::find_if(c.begin(), c.end(), [&](decltype(*c.end()) _){ return _.name == name; });
worse than
auto i = std::find_if(c.begin(), c.end(), MatchNameInFoo(name));
I need no other context to understand the first line, and it is simple enough that a bug is unlikely to creep in (and will get even simpler with polymorphic lambdas). Things like lambdas and bind exist so that people don't have to write those small classes by hand. Most people I know consider that a good thing.
I'd like the same thing for simple resource cleanup. I already have that when the resource is accessed via a pointer (unique_ptr); why not generalize it?
The problem isn't writing one single object; it's having to write, name and organize dozens to hundreds of different objects that do slightly different things.
Of course, if you can point me to dozens of code bases where people have embraced explicit RAII throughout their code, I'm certainly willing to change my opinion.
I personally would find it far more tedious to write little lambdas and such in every place I use a resource just to prevent resource leaks, when writing a single object once would solve the problem everywhere.
I personally would find it far more tedious to write little lambdas and such in every place I use a resource just to prevent resource leaks, when writing a single object once would solve the problem everywhere.I couldn't agree more with that. I personally believe that lambdas are worse for this kind of programming, i.e. they don't make code more readable. High abstraction is better than low abstraction, so how is this:
RAIIExitScope SomeNameIDontWantToConflictWithAnything([&] {fclose(pFile);}) //Note the lambdamore readable/maintanable to this:
RAIIExitScope SomeNameIDontWantToConflictWithAnything(safe_delete);//functor instead of lambdaHow can someone think that exposing guts (as lambdas do) is better than hiding them behind an abstraction level (as functors do)? This is simply not so.
--
I don't think contrived non-real-world examples are convincing. Especially the second example, the one of "loop advancement": isn't that what a for loop is for? (and then I can't stop thinking about iterators and the algorithms)
Martinho
I am open to suggestions.
I would be surprised to find out that other long time C++ programmers haven't seen similar problems with loop advancement -- because someone comes in a year after a product is shipping and while attempting to fix a bug adds a conditional continue to a loop which results in skipping over the loop advancement, producing a tight loop when that condition is hit. Sure, it can be avoided by code reviews and greater care, but by using RAII you have the ability to code a bit more defensively.
Some thoughts:
About the motivation and unique_ptr, I don't have to dereference a value from
.get(), I can just use operator* when I need a reference to the
underlying value.
If I need the pointer, I need .get(), sure. It's understandable that
these wrappers
have a cast operator, since they aren't similar to smart pointers in
that sense and
there's no danger in getting the underlying object.
Should scoped_resource throw from its constructor if it gets a no-delete value?
Should we have operator-> for accessing the underlying value?
The _uc and _t suffixes are unfortunately unclear. I'm not going to
start a bikeshedding
right now, but I think the names need to be reconsidered.
Should these handles have make_ function counterparts? That would probably
alleviate the need for the SCOPED_DELETER macro.
int *currValue = &someArray[0];
for(int i = 0; i < size; ++i)
{
scope(exit) {++currValue;} //Every iteration will update the current value.
//...
if(soemthing)
{
continue;
}
//++currValue; //old way: skipped by the continue. By accident.
}
int *currValue = &someArray[0];
for(int i = 0; i < size; ++i,++currValue
)
{
//...
if(soemthing)
{
continue;
}
}
For those willing to take a look at the proposal as a very rough draft, it can be found here: http://www.andrewlsandoval.com/scope_exit/
The Technical Specification section is completely blank. Judging by the examples (and by my understanding of them) this portion will probably take me several days. Of course, I'm grateful for any help, from anyone else that would like to see this functionality added to the Standard Library. And similarly for any feedback on the document as it currently stands. This is my first shot at this kind of thing! So be gentle please! ;)
void drive_any_available_bus()
{
auto &&bus = std::scope_exit(lock_available_bus(), unlock_bus); // Always unlock the bus when done driving... No need to verify the resource first
if(bus == -1)
{
return; // No buses...
}
drive(bus);
}
the following should do the same
void drive_any_available_bus()
{
if(lock_available_bus()
== -1) return;
auto &&bus = std::scope_exit(unlock_bus);
drive(bus);
}
The same applies to the third overload
// Open a file, ensure it is closed when we leave scope, if the file descriptor is valid...
auto&& iFileDesc = std::scope_exit(_open(pszPath, O_WRONLY), _close, -1); // if _open returns -1, don't call _close...
if(iFileDesc.valid())
{
_write(iFileDesc, &data, sizeof(data));
}
could be replaced by
auto fd = _open(pszPath, O_WRONLY);
if (fd != -1) return; // ...
auto&& guard = std::scope_exit( [&fd] { _close(fd); } );
_write(iFileDesc, &data, sizeof(data));
The last example concerns
a resource wrapper with implicit conversion to the underlying
resource. One of the problems with implicit conversions is that some
overloads can be missed. Resuming the simple scoped_exit is enough to make a useful C++14 proposal that covers all the examples you have on your current proposal and could have good chances to be accepted.class TestClassMember3 { private: FILE m_hEvent; public: TestClassMember2() : m_hEvent(CreateEvent(nullptr, TRUE, FALSE, nullptr)) { } ~TestClassMember() {
CloseHandle
(
m_hEvent
); } // Other stuff here... };
std::uncaught_exception
or any other mechanism so that failure or success conditions can be
determined.auto &&log_status_on_exit = std::scope_exit([&status, &modifier]() ->void
{
LogStatus(status, "SomeFunction", modifier);
});
scope(exit)
{
LogStatus(status, "SomeFunction", modifier);
}
On Tuesday, November 13, 2012 11:30:53 AM UTC-8, viboes wrote:
Le 13/11/12 08:48, Andrew Sandoval a écrit :
The proposal could be complemented with scoped_success, scoped_failure (based on D language). This could be possible updatingstd::uncaught_exception
or any other mechanism so that failure or success conditions can be determined.
That's the thing: if it's going to happen, I would much rather it be a language feature. Just compare the code:
auto &&log_status_on_exit = std::scope_exit([&status, &modifier]() ->void
{
LogStatus(status, "SomeFunction", modifier);
});
vs:
scope(exit)
{
LogStatus(status, "SomeFunction", modifier);
}
Isn't that so much better? No messy variables to create. No name collisions. No lambdas, and therefore no need to explicitly include anything in scope. It's immediately clear what it does.
That way, you don't need to modify std::uncaught_exception or anything like that. The system will call the `scope(failure)` at the correct time.
auto && _ = std::scope_exit(
[]() {
LogStatus(status, "SomeFunction", modifier);
});
On Tuesday, November 13, 2012 10:02:09 AM UTC-8, viboes wrote:
Le 13/11/12 18:47, Nicol Bolas a écrit :
I'm pretty sure he's talking about using RAII as the thing that increments the loop counter. Like this:
int *currValue = &someArray[0];
for(int i = 0; i < size; ++i)
{
scope(exit) {++currValue;} //Every iteration will update the current value.
//...
if(soemthing)
{
continue;
}
//++currValue; //old way: skipped by the continue. By accident.
}
Hi,
The C solution to this problem is to put the scoped exit action on the for, isn't it?
int *currValue = &someArray[0];
for(int i = 0; i < size; ++i,++currValue
)
{
//...
if(soemthing)
{
continue;
}
}
As I said, it would be used for more complex incrementing than something that could fit in a for-loop expression (or for non-for loops).
--You are surely right it could have a sense to use a scoped exit in this context, but then the example should include a non-for loop and/or a more complex increment or just remove it as it doesn't add nothing more.
> In my experience, every time an exception handler must be used, nesting and
> complexity increase, and often unnecessarily.
I have the completely opposite experience. I don't have to do nested checks
with exceptions, and the complexity of code is less than without exceptions.
I can decide where I put the complexity, and the no-error paths are fast and
clean because there's no checking of return values all over the place. That's
the reason people use RAII instead of return values or valid() checks.
>> Should we have operator-> for accessing the underlying value?
> I'm not opposed to that. I can add that if there is consensus. With the
> get() method, and a cast operator it doesn't feel intuitive to use
> operator-> to me, but I could be convinced.
When I use a handle, I want to use it for all accesses to the
underlying resource,
so if I have to call a member function of the underlying resource, I expect to
be able to do handle->func(), not handle.get().func() or, heaven forbid,
Resource& res = handle; res.func(); or Resource& res = handle.get(); res.func();
Handles are, as often as possible, as transparent as possible. That
would indicate
that we want to have, for the purposes of type-deducing function templates,
operator*(), and for things that want a pointer, operator&(), too.
I'm very appreciative of the feedback from many others that have helped move this along. I will try to find ways to make the examples more convincing, and I appreciate your suggestion. Please however, if you are on the committee and intend to shoot this down no matter what, please make that clear. This is a very time consuming process, and I don't have any to waste.
Okay, I've updated the reference implementation to include these operators and the option of throwing exception on validity failure. I've also changed all of the "generator" function templates to more closely follow the standard, so we now have make_scoped_function(), and make_scoped_resource().
I've updated draft of the proposal. It along with the links to the reference implementation are still at http://www.andrewlsandoval.com/scope_exit/.
Le 13/11/12 08:48, Andrew Sandoval a écrit :
Hi,
I don't think you need to add on your proposal the types returned by the scoped_exit function. Just propose a function that returns an implementation defined type that provides a specific behavior.
I'm for the first overloading, which corresponds to the interface Boost.ScopedExit has to make code portable.
The example motivating the second overload not convince me as we can use the simpler scoped_exit overload
Instead of
the following should do the same
void drive_any_available_bus() { auto &&bus = std::scope_exit(lock_available_bus(), unlock_bus); // Always unlock the bus when done driving... No need to verify the resource first if(bus == -1) { return; // No buses... } drive(bus); }
void drive_any_available_bus() { if(lock_available_bus()== -1) return;
drive(bus); }
auto &&bus = std::scope_exit(unlock_bus);
The same applies to the third overload
// Open a file, ensure it is closed when we leave scope, if the file descriptor is valid... auto&& iFileDesc = std::scope_exit(_open(pszPath, O_WRONLY), _close, -1); // if _open returns -1, don't call _close... if(iFileDesc.valid()) { _write(iFileDesc, &data, sizeof(data)); }
could be replaced by
if (fd != -1) return; // ...
auto fd = _open(pszPath, O_WRONLY);
auto&& guard = std::scope_exit( [&fd] { _close(fd); } );
_write(iFileDesc, &data, sizeof(data));
The last example concerns a resource wrapper with implicit conversion to the underlying resource. One of the problems with implicit conversions is that some overloads can be missed.
When wrapping a resource an alternative approach is to provide the specific functions, in your case define a File wrapper, but there is no need for a generic scoped_resource to implement it.
In particular the example could be implemented as
class TestClassMember3 { private: FILE m_hEvent; public: TestClassMember2() : m_hEvent(CreateEvent(nullptr, TRUE, FALSE, nullptr)) { } ~TestClassMember() {
CloseHandle
(
m_hEvent
); } // Other stuff here... };
Resuming the simple scoped_exit is enough to make a useful C++14 proposal that covers all the examples you have on your current proposal and could have good chances to be accepted.
Of course this doesn't covers all the classes you wanted to propose, but ...
The proposal could be complemented with scoped_success, scoped_failure (based on D language). This could be possible updatingstd::uncaught_exception
or any other mechanism so that failure or success conditions can be determined.
Just my 2 euro cents,
Vicente
Answers inline...
On Tuesday, November 13, 2012 1:30:53 PM UTC-6, viboes wrote:Le 13/11/12 08:48, Andrew Sandoval a écrit :
Hi,
I don't think you need to add on your proposal the types returned by the scoped_exit function. Just propose a function that returns an implementation defined type that provides a specific behavior.
I'm open to the idea that the implementation details can be implementation defined, but I don't see this example anywhere in the rest of the Standard Library. The library defines types and make_... functions for them, but doesn't just define make_... functions as far as I can tell.
I've made this easier in the updated reference implementation and proposal by using names that clearly distinguish the types, and "generator" function templates that use the make_... convention.
I'm for the first overloading, which corresponds to the interface Boost.ScopedExit has to make code portable.
I'm sorry, I don't understand what you are saying there.
The example motivating the second overload not convince me as we can use the simpler scoped_exit overload
Instead of
the following should do the same
void drive_any_available_bus() { auto &&bus = std::scope_exit(lock_available_bus(), unlock_bus); // Always unlock the bus when done driving... No need to verify the resource first if(bus == -1) { return; // No buses... } drive(bus); }
void drive_any_available_bus() { if(lock_available_bus()== -1) return;
drive(bus); }
auto &&bus = std::scope_exit(unlock_bus);Of course this will work, and this may be the best thing to do in certain cases, but if you code defensively you will bind the resource allocation to it's destruction, so that later on someone can't come in and add code that could throw an exception (for example) in between your allocation and your std::scope_exit() call.
The same applies to the third overload
// Open a file, ensure it is closed when we leave scope, if the file descriptor is valid... auto&& iFileDesc = std::scope_exit(_open(pszPath, O_WRONLY), _close, -1); // if _open returns -1, don't call _close... if(iFileDesc.valid()) { _write(iFileDesc, &data, sizeof(data)); }
could be replaced by
if (fd != -1) return; // ...
auto fd = _open(pszPath, O_WRONLY);
auto&& guard = std::scope_exit( [&fd] { _close(fd); } );
_write(iFileDesc, &data, sizeof(data));
Again, the same answer. Code it defensively. Don't assume that because this example is so simple it will never change. In real world code it is very likely that there will be other things happening that could be moved between your allocation (the call to _open) and the construction of guard. And it is even more likely that there will be code prior to the _open that does validation and constructs resources that will also need clean-up. By using RAII wrappers throughout you have the liberty of using the quick-exit returns throughout the function.
auto&& fd = _open(pszPath, O_WRONLY);
// do other things as throw an exception
auto&& iFileDesc = std::scope_exit(fd
, _close, -1);
if(iFileDesc.valid())
{
_write(iFileDesc, &data, sizeof(data));
}
);
Also, why declare a second named variable (guard), that never gets referenced? Bind the resource to its deleter-function and use it naturally throughout the code, without thinking again about it's clean-up.
auto&& ... = std::scope_exit( [&fd] { _close(fd); } );
or auto&& auto = std::scope_exit( [&fd] { _close(fd); } );
But this is a minor issue any way.
The last example concerns a resource wrapper with implicit conversion to the underlying resource. One of the problems with implicit conversions is that some overloads can be missed.
When wrapping a resource an alternative approach is to provide the specific functions, in your case define a File wrapper, but there is no need for a generic scoped_resource to implement it.
In particular the example could be implemented as
class TestClassMember3 { private: FILE m_hEvent; public: TestClassMember2() : m_hEvent(CreateEvent(nullptr, TRUE, FALSE, nullptr)) { } ~TestClassMember() {
CloseHandle
(
m_hEvent
); } // Other stuff here... };Again, in all of these examples you are breaking the tight coupling of the resource acquisition to its destruction.
By using std::unique_resource or std::scoped_resource as a class member variable, you ensure that destruction of the resource occurs in the proper order (the reverse order of initialization). You can eliminate the explicit destructor, which might be written unintentionally to clean-up out-of-order.
The goal here is to get you to change your whole paradigm. If you plan and declare the resource's lifetime when you initialize it or when you declare it (as is the case with std::unique_resource), you need not worry about remembering to put it in the destructor. It's easy to remember in a class as simple as TestClassMember2, but in real world code there will be much more to it, and the fact is that these things are forgotten all of the time.
std::unique_resource and std::scoped_resource also build in the validity checking. You can either call .valid() or you can use the throw option to throw failed_resource_initialization on construction of the wrapper.
Resuming the simple scoped_exit is enough to make a useful C++14 proposal that covers all the examples you have on your current proposal and could have good chances to be accepted.
Of course this doesn't covers all the classes you wanted to propose, but ...
Again, I'm not following what you are trying to say here. I've read it a couple of times and I must be missing something.
The proposal could be complemented with scoped_success, scoped_failure (based on D language). This could be possible updatingstd::uncaught_exception
or any other mechanism so that failure or success conditions can be determined.
That was added using the option to throw an exception on allocation / init failure. It needs to be optional however to cover the broadest use cases, such as environment where exceptions can't be tolerated.
Thanks Vicente. I hope I've answered your questions well.
Le 17/11/12 17:11, Andrew Sandoval a écrit :
Answers inline...std::bind ?
On Tuesday, November 13, 2012 1:30:53 PM UTC-6, viboes wrote:Le 13/11/12 08:48, Andrew Sandoval a écrit :
Hi,
I don't think you need to add on your proposal the types returned by the scoped_exit function. Just propose a function that returns an implementation defined type that provides a specific behavior.
I'm open to the idea that the implementation details can be implementation defined, but I don't see this example anywhere in the rest of the Standard Library. The library defines types and make_... functions for them, but doesn't just define make_... functions as far as I can tell.
I meant the the first overload of make_scoped_exit, which now corresponds to make_scoped_function.
I've made this easier in the updated reference implementation and proposal by using names that clearly distinguish the types, and "generator" function templates that use the make_... convention.
I'm for the first overloading, which corresponds to the interface Boost.ScopedExit has to make code portable.
I'm sorry, I don't understand what you are saying there.
I can understand the need for a wrapper that creates the resource on construction and release them on destruction. I just think the generic wrapper is not worth. At least your example should be much more explicit and from the real world.
The example motivating the second overload not convince me as we can use the simpler scoped_exit overload
Instead of
the following should do the same
void drive_any_available_bus() { auto &&bus = std::scope_exit(lock_available_bus(), unlock_bus); // Always unlock the bus when done driving... No need to verify the resource first if(bus == -1) { return; // No buses... } drive(bus); }
void drive_any_available_bus() { if(lock_available_bus()== -1) return;
drive(bus); }
auto &&bus = std::scope_exit(unlock_bus);Of course this will work, and this may be the best thing to do in certain cases, but if you code defensively you will bind the resource allocation to it's destruction, so that later on someone can't come in and add code that could throw an exception (for example) in between your allocation and your std::scope_exit() call.
The same applies to the third overload
// Open a file, ensure it is closed when we leave scope, if the file descriptor is valid... auto&& iFileDesc = std::scope_exit(_open(pszPath, O_WRONLY), _close, -1); // if _open returns -1, don't call _close... if(iFileDesc.valid()) { _write(iFileDesc, &data, sizeof(data)); }
could be replaced by
if (fd != -1) return; // ...
auto fd = _open(pszPath, O_WRONLY);
auto&& guard = std::scope_exit( [&fd] { _close(fd); } );
_write(iFileDesc, &data, sizeof(data));
Again, the same answer. Code it defensively. Don't assume that because this example is so simple it will never change. In real world code it is very likely that there will be other things happening that could be moved between your allocation (the call to _open) and the construction of guard. And it is even more likely that there will be code prior to the _open that does validation and constructs resources that will also need clean-up. By using RAII wrappers throughout you have the liberty of using the quick-exit returns throughout the function.
What prevents the user of allocating the resource outside of the scoped exit, as e.g. in
auto&& iFileDesc = std::scope_exit(fd
auto&& fd = _open(pszPath, O_WRONLY); // do other things as throw an exception, _close, -1); if(iFileDesc.valid()) { _write(iFileDesc, &data, sizeof(data)); }
SomeClass *pInstance = new SomeClass();
.
// Do stuff with pInstance
.
.
std::unique_ptr<SomeClass> pSomeClass(pInstance);
This is why I think that the wrapper must be specific
{
File f(pszPath, O_WRONLY);
// ...
} // close called on destruction
class File
{
private:
std::unique_resource<UNIQUE_DELETER(close), -1> m_iFd;
public:
File(const char *pszPath, int oflags, int iCreateFlags) : m_iFd(open(pszPath, oflags, iCreateFlags))
{
}
.
.
.
};
I would like to see a way to using anonymous variables
Also, why declare a second named variable (guard), that never gets referenced? Bind the resource to its deleter-function and use it naturally throughout the code, without thinking again about it's clean-up.
auto&& ... = std::scope_exit( [&fd] { _close(fd); } ); or
auto&& auto = std::scope_exit( [&fd] { _close(fd); } );But this is a minor issue any way.
Why? Here the class TestClassMemeber is playing the role of a specif wrapper.
The last example concerns a resource wrapper with implicit conversion to the underlying resource. One of the problems with implicit conversions is that some overloads can be missed.
When wrapping a resource an alternative approach is to provide the specific functions, in your case define a File wrapper, but there is no need for a generic scoped_resource to implement it.
In particular the example could be implemented as
class TestClassMember3 { private: FILE m_hEvent; public: TestClassMember2() : m_hEvent(CreateEvent(nullptr, TRUE, FALSE, nullptr)) { } ~TestClassMember() {
CloseHandle
(
m_hEvent
); } // Other stuff here... };Again, in all of these examples you are breaking the tight coupling of the resource acquisition to its destruction.
I understand that it is the role of the specific wrapper to release the resource :)By using std::unique_resource or std::scoped_resource as a class member variable, you ensure that destruction of the resource occurs in the proper order (the reverse order of initialization). You can eliminate the explicit destructor, which might be written unintentionally to clean-up out-of-order.
I don't think the generic classes you are proposing make the user to change to the best 'paradigm'. I really think the user needs specific wrappers, not these kind of generic raii classes that let him the responsibility to declare an auxiliary class to release the resource. A specific C++ class depending on the domain must be defined to avoid the user been working with non-safe resources. Well, this is just my opinion.
The goal here is to get you to change your whole paradigm. If you plan and declare the resource's lifetime when you initialize it or when you declare it (as is the case with std::unique_resource), you need not worry about remembering to put it in the destructor. It's easy to remember in a class as simple as TestClassMember2, but in real world code there will be much more to it, and the fact is that these things are forgotten all of the time.
I meant that IMO only the fist overload is useful. The resource abstractions are not. Proposing th scoped_exit function has a chance to be accepted.
std::unique_resource and std::scoped_resource also build in the validity checking. You can either call .valid() or you can use the throw option to throw failed_resource_initialization on construction of the wrapper.
Resuming the simple scoped_exit is enough to make a useful C++14 proposal that covers all the examples you have on your current proposal and could have good chances to be accepted.
Of course this doesn't covers all the classes you wanted to propose, but ...
Again, I'm not following what you are trying to say here. I've read it a couple of times and I must be missing something.
I think you have not understood my concern. What you have added lastly has nothing to be with scoped(success) scoped(failure). See the D language documentation.
The proposal could be complemented with scoped_success, scoped_failure (based on D language). This could be possible updatingstd::uncaught_exception
or any other mechanism so that failure or success conditions can be determined.
That was added using the option to throw an exception on allocation / init failure. It needs to be optional however to cover the broadest use cases, such as environment where exceptions can't be tolerated.
Now you did.
Thanks Vicente. I hope I've answered your questions well.
Vicente
Okay, I've updated the reference implementation to include these operators and the option of throwing exception on validity failure. I've also changed all of the "generator" function templates to more closely follow the standard, so we now have make_scoped_function(), and make_scoped_resource().
I've updated draft of the proposal. It along with the links to the reference implementation are still at http://www.andrewlsandoval.com/scope_exit/.
Hi Andrew,
On 18 November 2012 21:57, Nicol Bolas <jmck...@gmail.com> wrote:
> Besides, how is this all not a giant kludge? If you want this feature,
> propose it as a real, legitimate language feature, not some hack with
> lambdas and such. Because if it goes in as a hack, when someone does want it
> in the core language, they'll just say, "Use the hack."
What exactly do you mean by "this all"? Which part of the proposed library
facilities are "hacks"?
auto cleanup1 = make_cleanup( [&]() { <cleanup> } );
//Some code
auto cleanup1 = make_cleanup( [&]() { <more cleanup> } );
scope(exit) {<cleanup>}
//Some code
scope(exit) {<more cleanup>}
#include <cstdio>
#include <stdexcept>
template < typename Aquire, typename Release, typename Action >
inline auto bracket(Aquire aquire, Release release, Action action) -> decltype(action(aquire()))
{
auto val = aquire();
try
{
auto result = action(val);
release(val);
return result;
}
catch (...)
{
release(val);
throw ;
}
}
template < typename Action >
inline auto with_file(const char* filename, const char* mode, Action action) -> decltype(action(static_cast<FILE*>(nullptr)))
{
return bracket
(
[filename, mode]()
{
if (FILE* fp = std::fopen(filename, mode))
return fp;
else
throw std::runtime_error("Failed to open File.");
},
std::fclose,
action
);
}
int main()
{
return with_file("foo.txt", "r", [](FILE* fp)
{
// ...
return EXIT_SUCCESS;
});
}