Proposed new RAII Library
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII. Though shared_ptr, scoped_ptr, etc. greatly enhance the use of RAII, I propose two additional classes. The first of these is RAIIFunction (I'm open to other names and coding conventions). This one is very simple, but powerful. It's a small wrapper (using composition) around std::tr1::function, that simply calls the function on scope-exit, in the normal case. Unlike Boost.ScopeExit, it uses no macros. Anything that can be assigned to a std::tr1::function<AnyType> taking no parameters will work with it, so lambdas are perfect -- and allow for all of the associated benefits like capturing variables by reference, etc. It also works with std::tr1::bind / boost::bind, with no placeholders. // // RAIIFunction<> Written by Andrew L. Sandoval 2011/2012 // Include guard here #include <functional> // // An RAII class that works with std::tr1::bind and lambdas // NOTE: making a const RAIIFunction prevents the use of Invoke() // and Cancel() and operator=(fn) which is often appropriate // NOTE: m_func copies the original function object, so that const and // non-const refs can be passed in, etc. template <typename TRet> class RAIIFunction { private: std::tr1::function<TRet ()> m_func; bool m_bInvoke; RAIIFunction(); RAIIFunction(const RAIIFunction &); void operator=(const RAIIFunction &); public: RAIIFunction(const std::tr1::function<TRet ()> &fn) : m_func(fn), m_bInvoke(true) { } void operator=(const std::tr1::function<TRet ()> &fn) { if(m_bInvoke) { m_func(); } m_bInvoke = true; m_func = fn; } ~RAIIFunction() { if(m_bInvoke) { m_func(); } } void Cancel() { m_bInvoke = false; } TRet Invoke(bool bRelease = false) { m_bInvoke = !bRelease; return m_func(); } }; Examples: const RAIIFunction<void> closeDB = ([pDB]() { if(pDB) { sqlite3_close(pDB); } }); // // while example... bool bContinue = true; RECORD record = { }; while(bContinue) { // Ensure that no matter how the loop wraps (continue or fall-through) the next // record is grabbed or the loop terminates... const RAIIFunction<void> loopAdvance = ([&bContinue]() { bContinue = SomeCallThatGetsAnotherRecord(record); }); // A bunch of record processing here ... } // // A convoluted example that shows assignment, etc. RAIIFunction<int> mb = std::tr1::bind(MessageBoxW, HWND_TOP, L"Present pop-up prompt again?", L"Test", MB_TOPMOST|MB_YESNO); int iMB = mb.Invoke(true); if(IDYES != iMB) { mb = ([]() ->int { dprintf("Printing what the user didn't wanted popped up" " on scope-exit...\n"); return 0; }); -- The other one I call just RAIIWrapper, and it works with static deleter functions taking a single argument of the type being delete. Since it includes a lot more code (much of which could probably be replaced with type_traits code), I'll just show examples of usage below to gauge interest. I first used an earlier version of this in an environment where the standard function required a goto ErrorExit (like Microsoft example driver code in the WDK) to clean-up all of the resources. The original concern was that the c++ class would bloat the deliverable size. This class proved to be as tight as the goto equivalent. It uses one macro that I'll include below. The class takes 3 template parameters: 1) The type of the deleter function, 2) the deleter function, and 3) a no-delete value that defaults to NULL. // // Simplify usage with a macro: #define RAIIDeleter(deleterFunction) decltype(&deleterFunction), deleterFunction // Example: const RAIIWrapper<RAIIDeleter(CloseHandle), INVALID_HANDLE_VALUE> hFile = CreateFile(pwzFileName, GENERIC_WRITE, FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if(!hFile.IsValid()) { // handle error... } // Similar but using the default no-delete value (NULL): const RAIIWrapper<RAIIDeleter(CloseHandle)> hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); -- I've found that using RAII consistently can dramatically improve code quality. It forces the coder to think about resource lifetime at inception rather than at exit points, etc. Much of what I've shown above can be done with scoped_ptr or shared_ptr, but these specialized classes put the focus on RAII and resource management. Please let me know your thoughts. I'd love to see this become part of Boost, and hopefully eventually part of the C++ standard library. Thank You, Andrew Sandoval
On Sep 12, 2012 8:06 PM, "Andrew Sandoval" <sandoval@netwaysglobal.com> wrote:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII. Though shared_ptr, scoped_ptr, etc. greatly enhance the use of RAII, I propose two additional classes.
The first of these is RAIIFunction
[snip] I remember proposing something like this a few years back. My proposal was not based on boost:: or std::function for efficiency reasons. It used capturing the scope guard object by reference to a base class. The general idea is shown in the end of my post. I think, performance is a critical factor for tools like this. Also, have you looked at Boost.ScopeExit? It provides same functionality.
The other one I call just RAIIWrapper
[snip] I can't say I like the macro but the general idea looks interesting. I think the reference to base class trick can be used to hide the deleter type. Below is a code snippet (incomplete) illustrating the general idea I have. struct scope_guard_base { scope_guard_base() : active(true) {} bool active; }; template< typename F, typename Base = scope_guard_base > struct scope_guard_impl : Base { ~scope_guard_impl() { if (this->active) m_fun(); } private: F m_fun; }; typedef scope_guard_base const& scope_guard; template< typename F > scope_guard_impl< F > make_scope_guard(F fun) { return scope_guard_impl< F >(fun); } template< typename T > struct raii : scope_guard_base { T const& get() const { return m_obj; } private: T m_obj; }; template< typename T, typename F > scope_guard_impl< F, raii< T > > make_raii(T obj, F fun) { return scope_guard_impl< F, raii< T > >(fun, obj); } int main() { scope_guard guard = make_scope_guard(...); raii< HANDLE > const& handle = make_raii(CreateHandle(...), bind(CloseHandle, _1)); } With a little more work CloseHandle can be specified without bind (i.e. if you detect that the function accepts a single argument of type T).
On Tue, Sep 11, 2012 at 6:40 PM, Andrew Sandoval <sandoval@netwaysglobal.com> wrote:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII. Though shared_ptr, scoped_ptr, etc. greatly enhance the use of RAII, I propose two additional classes.
The first of these is RAIIFunction (I'm open to other names and coding conventions). This one is very simple, but powerful. It's a small wrapper (using composition) around std::tr1::function, that simply calls the function on scope-exit, in the normal case. Unlike Boost.ScopeExit, it uses no macros.
Seems similar to: http://www.boost.org/doc/libs/1_51_0/libs/scope_exit/doc/html/scope_exit/alt... #include <functional> struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; }; void world::add_person(person const& a_person) { bool commit = false; persons_.push_back(a_person); scope_exit on_exit1([&commit, this](void) { // Use C++11 lambda. if(!commit) persons_.pop_back(); // `persons_` via captured `this`. }); // ... commit = true; } I personally wouldn't need a lib for such a scope_exit class--it's trivial.
Anything that can be assigned to a std::tr1::function<AnyType> taking no parameters will work with it, so lambdas are perfect -- and allow for all of the associated benefits like capturing variables by reference, etc. It also works with std::tr1::bind / boost::bind, with no placeholders.
// // RAIIFunction<> Written by Andrew L. Sandoval 2011/2012 // Include guard here #include <functional>
// // An RAII class that works with std::tr1::bind and lambdas // NOTE: making a const RAIIFunction prevents the use of Invoke() // and Cancel() and operator=(fn) which is often appropriate // NOTE: m_func copies the original function object, so that const and // non-const refs can be passed in, etc. template <typename TRet> class RAIIFunction { private: std::tr1::function<TRet ()> m_func; bool m_bInvoke;
RAIIFunction(); RAIIFunction(const RAIIFunction &); void operator=(const RAIIFunction &); public: RAIIFunction(const std::tr1::function<TRet ()> &fn) : m_func(fn), m_bInvoke(true) { }
void operator=(const std::tr1::function<TRet ()> &fn) { if(m_bInvoke) { m_func(); } m_bInvoke = true; m_func = fn; }
~RAIIFunction() { if(m_bInvoke) { m_func(); } }
void Cancel() { m_bInvoke = false; }
TRet Invoke(bool bRelease = false) { m_bInvoke = !bRelease; return m_func(); } };
Examples: const RAIIFunction<void> closeDB = ([pDB]() { if(pDB) { sqlite3_close(pDB); } });
// // while example... bool bContinue = true; RECORD record = { }; while(bContinue) { // Ensure that no matter how the loop wraps (continue or fall-through) the next // record is grabbed or the loop terminates... const RAIIFunction<void> loopAdvance = ([&bContinue]() { bContinue = SomeCallThatGetsAnotherRecord(record); });
// A bunch of record processing here ... }
// // A convoluted example that shows assignment, etc. RAIIFunction<int> mb = std::tr1::bind(MessageBoxW, HWND_TOP, L"Present pop-up prompt again?", L"Test", MB_TOPMOST|MB_YESNO); int iMB = mb.Invoke(true); if(IDYES != iMB) { mb = ([]() ->int { dprintf("Printing what the user didn't wanted popped up" " on scope-exit...\n"); return 0; });
--
The other one I call just RAIIWrapper, and it works with static deleter functions taking a single argument of the type being delete. Since it includes a lot more code (much of which could probably be replaced with type_traits code), I'll just show examples of usage below to gauge interest. I first used an earlier version of this in an environment where the standard function required a goto ErrorExit (like Microsoft example driver code in the WDK) to clean-up all of the resources. The original concern was that the c++ class would bloat the deliverable size. This class proved to be as tight as the goto equivalent. It uses one macro that I'll include below. The class takes 3 template parameters: 1) The type of the deleter function, 2) the deleter function, and 3) a no-delete value that defaults to NULL.
// // Simplify usage with a macro: #define RAIIDeleter(deleterFunction) decltype(&deleterFunction), deleterFunction
// Example: const RAIIWrapper<RAIIDeleter(CloseHandle), INVALID_HANDLE_VALUE> hFile = CreateFile(pwzFileName, GENERIC_WRITE, FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if(!hFile.IsValid()) { // handle error... }
// Similar but using the default no-delete value (NULL): const RAIIWrapper<RAIIDeleter(CloseHandle)> hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
--
I've found that using RAII consistently can dramatically improve code quality. It forces the coder to think about resource lifetime at inception rather than at exit points, etc. Much of what I've shown above can be done with scoped_ptr or shared_ptr, but these specialized classes put the focus on RAII and resource management.
Please let me know your thoughts. I'd love to see this become part of Boost, and hopefully eventually part of the C++ standard library.
HTH, --Lorenzo
On 12/09/2012 2:56 PM, Klaim - Joël Lamotte wrote:
On Wed, Sep 12, 2012 at 8:09 PM, Lorenzo Caminiti<lorcaminiti@gmail.com>wrote:
I personally wouldn't need a lib for such a scope_exit class--it's trivial.
If you make it standard or boost library, it's even more trivial.
Joel Lamotte
+1 (also, the demo scope_exit linked needs a dismiss function)
Lorenzo Caminiti <lorcaminiti <at> gmail.com> writes:
Seems similar to:
http://www.boost.org/doc/libs/1_51_0/libs/scope_exit/doc/html/scope_exit/alt...
#include <functional>
struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; };
void world::add_person(person const& a_person) { bool commit = false;
persons_.push_back(a_person); scope_exit on_exit1([&commit, this](void) { // Use C++11 lambda. if(!commit) persons_.pop_back(); // `persons_` via captured `this`. });
// ...
commit = true; }
I personally wouldn't need a lib for such a scope_exit class--it's trivial.
Yes, it is very similar to the scope_exit 'alternative' example. (The keyword there is example, and you have to hunt to find this example.) Yes, it is trivial to write, but so is boost::noncopyable and because it exists in the library it gets used and re-used instead of being reinvented over and over again. (Isn't that the point to a library?) Regardless of how trivial it may be, making it a standard will increase the use of RAII and thereby increase code quality in many cases. The fact that it is simple will mean more engineers will learn to use it and improve not just the code they write, but the way they think while writting code -- because employing RAII in my experience has that side benefit. When you contemplate a resource's lifetime at inception you reduce a lot of common mistakes, like holding locks longer than necessary, leaking objects/memory, etc. Thanks, Andrew Sandoval
Andrew Sandoval <sandoval <at> netwaysglobal.com> writes:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII.
So, not to be impatient, but where do we go from here? It sounds like a few folks agree and like the idea, one person think he can do without this, but how does a proposal like this get traction and make it in to Boost? And, should I also post the other class here? Thanks, Andrew Sandoval
On 13/09/2012 10:05 PM, Andrew Sandoval wrote:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII. So, not to be impatient, but where do we go from here? It sounds like a few folks agree and like the idea, one person think he can do without this, but how does a proposal like this get traction and make it in to Boost?
And, should I also post the other class here?
I think you should find someone who is willing to commit it to svn and then submit a patch that they can apply to boost trunk including tests and documentation. I can help if you want. Probably not actually. But in spirit. Sohail
On Thursday 13 September 2012 23:15:44 Sohail Somani wrote:
On 13/09/2012 10:05 PM, Andrew Sandoval wrote:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII.
So, not to be impatient, but where do we go from here? It sounds like a few folks agree and like the idea, one person think he can do without this, but how does a proposal like this get traction and make it in to Boost?
And, should I also post the other class here?
I think you should find someone who is willing to commit it to svn and then submit a patch that they can apply to boost trunk including tests and documentation.
I can help if you want. Probably not actually. But in spirit.
Actually, there's this procedure: http://www.boost.org/development/submissions.html Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this: http://www.boost.org/development/requirements.html
On Thu, Sep 13, 2012 at 10:36 PM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
On Thursday 13 September 2012 23:15:44 Sohail Somani wrote:
On 13/09/2012 10:05 PM, Andrew Sandoval wrote:
I'd like to propose a new Boost library for RAII wrapper classes that I think could be standardized to help promote the use of RAII.
So, not to be impatient, but where do we go from here? It sounds like a few folks agree and like the idea, one person think he can do without this, but how does a proposal like this get traction and make it in to Boost?
And, should I also post the other class here?
I think you should find someone who is willing to commit it to svn and then submit a patch that they can apply to boost trunk including tests and documentation.
I can help if you want. Probably not actually. But in spirit.
Actually, there's this procedure:
Also I personally found it useful to have submitted reviews for 3 lib before having Boost review my 1st lib (but that is /not/ a requirement for a lib author).
Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this:
A couple of questions that will likely come up during the review: 1. How is the lib different from Scope Exit? 2. Shall the lib be added as an extension to Scope Exit? 3. Is the lib useful also on C++03 or only on C++11? HTH, --Lorenzo
On 14/09/2012 1:46 AM, Lorenzo Caminiti wrote:
A couple of questions that will likely come up during the review:
1. How is the lib different from Scope Exit?
I think it adds missing functionality.
2. Shall the lib be added as an extension to Scope Exit?
Yeah, this should quietly be added as part of scope exit. DON'T TELL ANYONE. If the author agrees, I suppose.
3. Is the lib useful also on C++03 or only on C++11?
Very useful in both. I'M TIRED OF WRITING THIS CLASS. But I'll keep doing it if I have to. Sohail
On 14/09/2012 1:36 AM, Andrey Semashev wrote:
Actually, there's this procedure:
http://www.boost.org/development/submissions.html
Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this:
My view is that it's a missing feature in the existing Scope Exit library.
On Friday 14 September 2012 09:07:07 Sohail Somani wrote:
On 14/09/2012 1:36 AM, Andrey Semashev wrote:
Actually, there's this procedure:
http://www.boost.org/development/submissions.html
Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this: My view is that it's a missing feature in the existing Scope Exit library.
I don't think so. I've been using Boost.ScopeExit for quite a while and don't really miss the scope guard function (like the proposed RAIIFunction). If it is proposed it has to provide something remarkably different or benefical, IMHO. As for the RAII wrapper, this component may well deserve its own mini-library.
On 14/09/2012 9:22 AM, Andrey Semashev wrote:
On Friday 14 September 2012 09:07:07 Sohail Somani wrote:
On 14/09/2012 1:36 AM, Andrey Semashev wrote:
Actually, there's this procedure:
http://www.boost.org/development/submissions.html
Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this: My view is that it's a missing feature in the existing Scope Exit library.
I don't think so. I've been using Boost.ScopeExit for quite a while and don't really miss the scope guard function (like the proposed RAIIFunction). If it is proposed it has to provide something remarkably different or benefical, IMHO.
As for the RAII wrapper, this component may well deserve its own mini-library.
A proposed library should be more than a tweak on an existing idea that fits just fine with an existing library. BOOST_SCOPE_EXIT is way overkill for simple handle closing or transaction commit operations: BOOST_SCOPE_EXIT( (&handle) ) { CloseHandle(handle); } vs boost::scope_exit OMGINEEDANAME(CloseHandle,handle); I'd even be OK with: BOOST_SCOPE_EXIT(CloseHandle,handle); Too many curly braces in my code already. I need more :: and BOOST_* Sohail
On Friday 14 September 2012 10:56:46 Sohail Somani wrote:
On 14/09/2012 9:22 AM, Andrey Semashev wrote:
As for the RAII wrapper, this component may well deserve its own mini-library.
A proposed library should be more than a tweak on an existing idea that fits just fine with an existing library. BOOST_SCOPE_EXIT is way overkill for simple handle closing or transaction commit operations:
BOOST_SCOPE_EXIT( (&handle) ) { CloseHandle(handle); }
vs
boost::scope_exit OMGINEEDANAME(CloseHandle,handle);
I'd even be OK with:
BOOST_SCOPE_EXIT(CloseHandle,handle);
Too many curly braces in my code already. I need more :: and BOOST_*
Your last two examples fit the "RAII wrapper" I was referring to. But I don't think this wrapper belongs to Boost.ScopeExit (or, rather, it doesn't deserve own library). What if I want to keep that handle as a class member (and, naturally, call CloseHandle when destructor is called)? You could argue that the call to CloseHandle could be hardcoded in my class' destructor but that is inconvinient an error prone. Why not: typedef raii< HANDLE, &CloseHandle > auto_handle; class my_class { auto_handle m_h; }; This doesn't look like a part of Boost.ScopeExit anymore.
On 14/09/2012 11:12 AM, Andrey Semashev wrote:
typedef raii< HANDLE, &CloseHandle > auto_handle;
class my_class { auto_handle m_h; };
This doesn't look like a part of Boost.ScopeExit anymore.
I could make a car into an airplane and it's not a car anymore. This is quite clever but limited. Sohail
On Friday 14 September 2012 11:59:15 Sohail Somani wrote:
On 14/09/2012 11:12 AM, Andrey Semashev wrote:
typedef raii< HANDLE, &CloseHandle > auto_handle;
class my_class {
auto_handle m_h;
};
This doesn't look like a part of Boost.ScopeExit anymore.
I could make a car into an airplane and it's not a car anymore.
This is quite clever but limited.
How so? I'd say the opposite.
Sohail Somani <sohail <at> taggedtype.net> writes:
On 14/09/2012 11:12 AM, Andrey Semashev wrote:
typedef raii< HANDLE, &CloseHandle > auto_handle;
class my_class { auto_handle m_h; };
This doesn't look like a part of Boost.ScopeExit anymore.
I could make a car into an airplane and it's not a car anymore.
This is quite clever but limited.
How so? I use my RAII class like that all of the time, and it does exactly what is needed. It guarantees I'll never forget to put the CloseHandle into the destructor. In fact, most of the time it means I don't need destructors anymore. -Andrew Sandoval
On 14/09/2012 2:24 PM, Andrew Sandoval wrote:
On 14/09/2012 11:12 AM, Andrey Semashev wrote:
typedef raii< HANDLE, &CloseHandle > auto_handle;
class my_class { auto_handle m_h; };
This doesn't look like a part of Boost.ScopeExit anymore.
I could make a car into an airplane and it's not a car anymore.
This is quite clever but limited.
How so? I use my RAII class like that all of the time, and it does exactly what is needed. It guarantees I'll never forget to put the CloseHandle into the destructor. In fact, most of the time it means I don't need destructors anymore.
Oh, it's limited because you can't include any state in the deallocation: void youLikeThis(bool doYou,Handle h); boost::scope_guard OMGANAME(youLikeThis,true,handle); Not possible if everything is encoded in the type. Sohail
Sohail Somani <sohail <at> taggedtype.net> writes:
Oh, it's limited because you can't include any state in the deallocation:
void youLikeThis(bool doYou,Handle h);
boost::scope_guard OMGANAME(youLikeThis,true,handle);
Not possible if everything is encoded in the type.
True, but that is why I was proposing two RAII classes. One that is limited and uses only static values for the deleter function and the no-delete value, and another that is much more open including state (RAIIFunction) because you can use bind with it, and lambda's that provide any needed state. -Andrew Sandoval
On 14/09/2012 2:58 PM, Andrew Sandoval wrote:
True, but that is why I was proposing two RAII classes. One that is limited and uses only static values for the deleter function and the no-delete value, and another that is much more open including state (RAIIFunction) because you can use bind with it, and lambda's that provide any needed state.
Ok, I guess you like the one with it encoded in the type. I can see its value. Regarding the second kind: https://github.com/ryppl/boost-svn/blob/master/boost/scope_exit.hpp#L615 Yes, it's already there but hidden in aux or detali or something. operator= is a more general dismiss(). So just making it public would be great. Sohail
Le 14/09/12 21:12, Sohail Somani a écrit :
On 14/09/2012 2:58 PM, Andrew Sandoval wrote:
True, but that is why I was proposing two RAII classes. One that is limited and uses only static values for the deleter function and the no-delete value, and another that is much more open including state (RAIIFunction) because you can use bind with it, and lambda's that provide any needed state.
Ok, I guess you like the one with it encoded in the type. I can see its value.
Regarding the second kind:
https://github.com/ryppl/boost-svn/blob/master/boost/scope_exit.hpp#L615
Yes, it's already there but hidden in aux or detali or something.
operator= is a more general dismiss().
So just making it public would be great. I don't like that the destructor checks for f_ in the preceding link.
template<> struct guard<void> { ~guard() { if(f_) f_(); } template<typename Lambda> void operator=(Lambda f) { f_ = f; } private: boost::function<void (void)> f_; }; And I don't know if the assignment operator should be exposed to the user, even if it could have no cost. Anyway the current implementation could require that the parameter should be Callable. The null pattern should be applied there and assigning a lambda that do nothing will avoid the check. BTW, couldn't the cost of boost::function be reduced using the reference to a base class technique used in multi_index/detail/scope_guard.hpp and described in Alexandrescu, A., Marginean, P.:"Generic<Programming>: Change the Way You Write Exception-Safe Code - Forever", C/C++ Users Jornal, Dec 2000, http://www.drdobbs.com/184403758 Best, Vicente
Andrew Sandoval <sandoval <at> netwaysglobal.com> writes:
On 14/09/2012 11:12 AM, Andrey Semashev wrote:
typedef raii< HANDLE, &CloseHandle > auto_handle; class my_class { auto_handle m_h; };
I might add that I much prefer to not typedef RAII classes away like that shown in the example above. If I'm putting it into another class, I like to be able to see exactly how the resource is being dealt with. For example: class Something { private: RAIIWrapper<RAIIDeleter(CloseHandle),INVALID_HANDLE_VALUE> m_hFile; RAIIWrapper<RAIIDeleter(CloseHandle)> m_hEvent; public: Something() : m_hFile(OpenSomethingFile()), m_hEvent(CreateEvent(NULL, TRUE, FALSE, NULL)) { } bool IsFileOpen() const { return m_hFile.IsValid(); } . . . }; I could typedef those two into separate auto_handle's, one for each no-delete value, but having it front and center like that IMO improves understanding of what it is used for. -Andrew Sandoval
On Fri, Sep 14, 2012 at 7:56 AM, Sohail Somani <sohail@taggedtype.net> wrote:
On 14/09/2012 9:22 AM, Andrey Semashev wrote:
On Friday 14 September 2012 09:07:07 Sohail Somani wrote:
On 14/09/2012 1:36 AM, Andrey Semashev wrote:
Actually, there's this procedure:
http://www.boost.org/development/submissions.html
Specifically, a formal review is required before the library can be committed. Also, the library author should probably read this:
My view is that it's a missing feature in the existing Scope Exit library.
I don't think so. I've been using Boost.ScopeExit for quite a while and don't really miss the scope guard function (like the proposed RAIIFunction). If it is proposed it has to provide something remarkably different or benefical, IMHO.
As for the RAII wrapper, this component may well deserve its own mini-library.
A proposed library should be more than a tweak on an existing idea that fits just fine with an existing library. BOOST_SCOPE_EXIT is way overkill for simple handle closing or transaction commit operations:
BOOST_SCOPE_EXIT( (&handle) ) { CloseHandle(handle); }
vs
boost::scope_exit OMGINEEDANAME(CloseHandle,handle);
I'd even be OK with:
BOOST_SCOPE_EXIT(CloseHandle,handle);
Please consider programming: #define UNARY_SCOPE_EXIT(unary_func, arg) \ BOOST_SCOPE_EXIT( (&arg) ) { \ unary_func(arg); \ } BOOST_SCOPE_EXIT_END And then: UNARY_SCOPE_EXIT(CloseHandle, handle); That way the code for this one specialized use case of Scope Exit becomes closer to what your expressed preference is. You can of course put this macro #definition into your own little lib scope_exit_arity.hpp and re-use it across your different projects :)
Too many curly braces in my code already. I need more :: and BOOST_*
HTH, --Lorenzo
On 14/09/2012 11:43 AM, Lorenzo Caminiti wrote:
UNARY_SCOPE_EXIT(CloseHandle, handle);
That way the code for this one specialized use case of Scope Exit becomes closer to what your expressed preference is. You can of course put this macro #definition into your own little lib scope_exit_arity.hpp and re-use it across your different projects:)
This was pretty clever and I exclaimed "mother of god" but honestly I need the scope guard's "dismiss/release" function as does 99.93% of people who want the scope guard (I just did a poll.) SO_CLOSE. Sohail
Sohail Somani wrote:
On 14/09/2012 11:43 AM, Lorenzo Caminiti wrote:
UNARY_SCOPE_EXIT(CloseHandle, handle);
That way the code for this one specialized use case of Scope Exit becomes closer to what your expressed preference is. You can of course put this macro #definition into your own little lib scope_exit_arity.hpp and re-use it across your different projects:)
This was pretty clever and I exclaimed "mother of god" but honestly I need the scope guard's "dismiss/release" function as does 99.93% of people who want the scope guard (I just did a poll.)
#define UNARY_SCOPE_EXIT(dismiss, unary_func, arg) \ BOOST_SCOPE_EXIT( (&dismiss)(&arg) ) { \ if (!dismiss) unary_func(arg); \ } BOOST_SCOPE_EXIT_END And then: bool dismiss = false; ... UNARY_SCOPE_EXIT(dismiss, CloseHandle, handle); ... dismiss = true; Alex
On 14/09/2012 2:37 PM, Alexander Nasonov wrote:
And then:
bool dismiss = false;
...
UNARY_SCOPE_EXIT(dismiss, CloseHandle, handle);
...
dismiss = true;
What. Are you really trying to make a case against: boost::scope_guard kanyewest(CloseHandle,handle); ... kanyewest.dismiss(); Or in C++11: boost::scope_guard lolwhat( [&](){ CloseHandle(handle); } ); ... lolwhat.dismiss(); Macros are magic and I concur with the other poster that unless the magic cannot be avoided, don't rely on it. I'm not sure why this is so hard. People who use this pattern find it more useful that BOOST_SCOPE_EXIT. People who don't, don't. Film at 11. Sohail
Le 14/09/12 20:48, Sohail Somani a écrit :
On 14/09/2012 2:37 PM, Alexander Nasonov wrote:
And then:
bool dismiss = false;
...
UNARY_SCOPE_EXIT(dismiss, CloseHandle, handle);
...
dismiss = true;
What. Are you really trying to make a case against:
boost::scope_guard kanyewest(CloseHandle,handle);
... kanyewest.dismiss();
Or in C++11:
boost::scope_guard lolwhat( [&](){ CloseHandle(handle); } );
... lolwhat.dismiss();
Yes, I'm. Encapsulating the dismiss condition on the scope_guard doesn't scale when you have several actions that need to be executed. Compare bool dismiss = false; ... UNARY_SCOPE_EXIT(dismiss, CloseHandle, handle1); UNARY_SCOPE_EXIT(dismiss, CloseHandle, handle2); ... dismiss = true; with boost::scope_guard lolwhat1( [&](){ CloseHandle(handle1); } ); boost::scope_guard lolwhat2( [&](){ CloseHandle(handle2); } ); ... lolwhat1.dismiss(); lolwhat2.dismiss(); Not only you need to dismiss twice, but you need to name differently your scope guards and don't forget to dismiss all of them.
Macros are magic and I concur with the other poster that unless the magic cannot be avoided, don't rely on it. You are right. Macros should be avoided when possible. The interface of your class should be modified to take care of an external dismiss condition.
bool dismiss = false; ... boost::dismissed_scope_guard lolwhat1(dismiss, [&](){ CloseHandle(handle1); } ); boost::dismissed_scope_guard lolwhat2(dismiss, [&](){ CloseHandle(handle2); } ); ... dismiss = true; Or passing it to the ClocseHandle function bool dismiss = false; ... boost::scope_guard lolwhat1( [&](){ CloseHandleIfNot(dismiss,handle1); } ); boost::scope_guard lolwhat2( [&](){ CloseHandleIfNot(dismiss,handle2); } ); ... dismiss = true; or to the lambda bool dismiss = false; ... boost::scope_guard lolwhat1( [&](){ if (!dismiss) CloseHandle(handle1); } ); boost::scope_guard lolwhat2( [&](){ if (!dismiss) CloseHandle(handle2); } ); ... dismiss = true; which I prefer, as in this case, scope_guard doesn't provide unnecessary behavior and I don't need to define an new function that takes care of the dismiss condition. In addition, the problem is that sometimes the dismiss condition is not stored in a variable, so that mandating the use of one is not desired.
I'm not sure why this is so hard. People who use this pattern find it more useful that BOOST_SCOPE_EXIT. People who don't, don't. Film at 11.
While this non-macro interface is possible only with C++11 lambdas, there is no portable way to do it (as generic as the last usage) in C++03. We need the pre-processor to have the same functionality, and this was the motivation of Boost.ScopeExit. So resuming, the boost::scope_guard (without a dismiss state) is a convenient abstraction for c++11 compilers supporting lambdas, and Boost.ScopeExit reflects the same abstraction for C++03. I will be for an extension of Boost.ScopeExit that provides a non-macro interface, like the one of the preceding scope_guard, for the people that don't need to make their code portable to non c++11 compilers. I will name it however scoped_exit. Extracted from the Boost.ScopeExit struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; }; bool dismiss = false; ... boost::scope_exit lolwhat1( [&](){ if (!dismiss) CloseHandle(handle1); } ); boost::scope_exit lolwhat2( [&](){ if (!dismiss) CloseHandle(handle2); } ); ... dismiss = true; I don't know if I will use in my code the proposed scope_guard interface having as parameters a function and its arguments, as in boost::scope_guard kanyewest(CloseHandleIfNot, dismiss, handle); Best, Vicente P.S.. The lambdas are missing some parameters, as dismiss, handle, ...
Vicente J. Botet Escriba <vicente.botet <at> wanadoo.fr> writes:
I don't know if I will use in my code the proposed scope_guard interface having as parameters a function and its arguments, as in
boost::scope_guard kanyewest(CloseHandleIfNot, dismiss, handle);
Vincente: I am a little confused. Are you suggesting that you will write these new classes, or that I should put this in mine? The one I sent originally has a dismiss method, it's just called Cancel(). It also has operator=. And while it would only work with C++11 compilers using lambdas, if you were using boost::bind it would work with C++03 compilers as well. Since I proposed this I will work on changing my two classes (RAIIWrapper and RAIIFunction) so that they comply better with the C++ Library standards (nomenclature, etc.) and I will submit that for review. I am completely open however to putting it an existing library, though I think it might help if we Boost had an RAII library that contained these two classes AND in the documentation links to other RAII containers like scoped_ptr, shared_ptr, and Boost.ScopedExit. -Andrew Sandoval
On Fri, Sep 14, 2012 at 2:02 PM, Andrew Sandoval <sandoval@netwaysglobal.com> wrote:
Vicente J. Botet Escriba <vicente.botet <at> wanadoo.fr> writes:
I don't know if I will use in my code the proposed scope_guard interface having as parameters a function and its arguments, as in
boost::scope_guard kanyewest(CloseHandleIfNot, dismiss, handle);
Vincente: I am a little confused. Are you suggesting that you will write these new classes, or that I should put this in mine? The one I sent originally has a dismiss method, it's just called Cancel(). It also has operator=.
And while it would only work with C++11 compilers using lambdas, if you were using boost::bind it would work with C++03 compilers as well.
No, it won't because with C++11 you can execute *arbitrary* copy at scope exit while with bind you can execute a function with arbitrary arity but still just execute a function a scope exit. How do I state this more clearly?? Well say with bind you can't do: if (...) { f(); int x = 1; } else {...} for (...) ... (Phoenix might help but that's like trying to use lamba expressions on C++03.)
Since I proposed this I will work on changing my two classes (RAIIWrapper and RAIIFunction) so that they comply better with the C++ Library standards (nomenclature, etc.) and I will submit that for review. I am completely open however to putting it an existing library, though I think it might help if we Boost had an RAII library that contained these two classes AND in the documentation links to other RAII containers like scoped_ptr, shared_ptr, and Boost.ScopedExit.
Really HTH, --Lorenzo
Lorenzo Caminiti <lorcaminiti <at> gmail.com> writes:
No, it won't because with C++11 you can execute *arbitrary* copy at scope exit while with bind you can execute a function with arbitrary arity but still just execute a function a scope exit. How do I state this more clearly?? Well say with bind you can't do:
if (...) { f(); int x = 1; } else {...} for (...) ...
(Phoenix might help but that's like trying to use lamba expressions on C++03.)
I think you are misunderstanding me. I'm not saying it will work as a lambda. I'm saying that RAIIFunction will work with boost::bind / std::tr1::bind. In other words, it is useful in C++03, not just C++11. -Andrew Sandoval
On Fri, Sep 14, 2012 at 1:08 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
So resuming, the boost::scope_guard (without a dismiss state) is a convenient abstraction for c++11 compilers supporting lambdas, and Boost.ScopeExit reflects the same abstraction for C++03.
I will be for an extension of Boost.ScopeExit that provides a non-macro interface, like the one of the preceding scope_guard, for the people that don't need to make their code portable to non c++11 compilers. I will name it however scoped_exit.
Extracted from the Boost.ScopeExit
struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; };
void world::add_person(person const& a_person) { bool commit = false; persons_.push_back(a_person); scope_exit on_exit1([&commit, this](void) { // Use C++11 lambda. if(!commit) persons_.pop_back(); // `persons_` via captured `this`. }); // ... commit = true; } Sure if there's enough interest I can move this scope_exit class from the examples into the Scope Exit lib itself when #ifndef BOOST_NO_LAMBDAS. My _personal_ opinion is that such a boost::scope_exit class is so trivial that programmers can just program it themselves, but that's just my opinion and I'm happy to add such a feature (or better to move it into the lib from the examples) if Boosters want it. No review should be needed, I can just post the (trivial) changes on this ML and confirm them with Alexander Nasonov. Thanks. --Lorenzo
Le 15/09/12 00:28, Lorenzo Caminiti a écrit :
On Fri, Sep 14, 2012 at 1:08 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
So resuming, the boost::scope_guard (without a dismiss state) is a convenient abstraction for c++11 compilers supporting lambdas, and Boost.ScopeExit reflects the same abstraction for C++03.
I will be for an extension of Boost.ScopeExit that provides a non-macro interface, like the one of the preceding scope_guard, for the people that don't need to make their code portable to non c++11 compilers. I will name it however scoped_exit.
Extracted from the Boost.ScopeExit
struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; }; void world::add_person(person const& a_person) { bool commit = false;
persons_.push_back(a_person); scope_exit on_exit1([&commit, this](void) { // Use C++11 lambda. if(!commit) persons_.pop_back(); // `persons_` via captured `this`. });
// ...
commit = true; }
Sure if there's enough interest I can move this scope_exit class from the examples into the Scope Exit lib itself when #ifndef BOOST_NO_LAMBDAS.
My _personal_ opinion is that such a boost::scope_exit class is so trivial that programmers can just program it themselves, but that's just my opinion and I'm happy to add such a feature (or better to move it into the lib from the examples) if Boosters want it. No review should be needed, I can just post the (trivial) changes on this ML and confirm them with Alexander Nasonov. Lorenzo, thanks for volunteering but the design and implementation is far more complex than the extract from the doc. If we want it to be efficient, it shouldn't use boost.function, boost/multi_index/detail/scope_guard.hpp should be a good staring point.
Best, Vicente
On Fri, Sep 14, 2012 at 4:19 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 15/09/12 00:28, Lorenzo Caminiti a écrit :
On Fri, Sep 14, 2012 at 1:08 PM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
So resuming, the boost::scope_guard (without a dismiss state) is a convenient abstraction for c++11 compilers supporting lambdas, and Boost.ScopeExit reflects the same abstraction for C++03.
I will be for an extension of Boost.ScopeExit that provides a non-macro interface, like the one of the preceding scope_guard, for the people that don't need to make their code portable to non c++11 compilers. I will name it however scoped_exit.
Extracted from the Boost.ScopeExit
struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; };
void world::add_person(person const& a_person) { bool commit = false;
persons_.push_back(a_person); scope_exit on_exit1([&commit, this](void) { // Use C++11 lambda. if(!commit) persons_.pop_back(); // `persons_` via captured `this`. });
// ...
commit = true; }
Sure if there's enough interest I can move this scope_exit class from the examples into the Scope Exit lib itself when #ifndef BOOST_NO_LAMBDAS.
My _personal_ opinion is that such a boost::scope_exit class is so trivial that programmers can just program it themselves, but that's just my opinion and I'm happy to add such a feature (or better to move it into the lib from the examples) if Boosters want it. No review should be needed, I can just post the (trivial) changes on this ML and confirm them with Alexander Nasonov.
Lorenzo, thanks for volunteering but the design and implementation is far more complex than the extract from the doc. If we want it to be efficient, it shouldn't use boost.function, boost/multi_index/detail/scope_guard.hpp should be a good staring point.
I see. Then (speaking personally) it might make sense for it to exist in a lib as its impl is not trivial due to the optimization efforts. However, I probably won't have time to add this feature to Scope Exit... Thanks. --Lorenzo
On 15/09/2012 2:16 AM, Lorenzo Caminiti wrote:
Lorenzo, thanks for volunteering but the design and implementation is far more complex than the extract from the doc. If we want it to be efficient, it shouldn't use boost.function, boost/multi_index/detail/scope_guard.hpp should be a good staring point. I see. Then (speaking personally) it might make sense for it to exist in a lib as its impl is not trivial due to the optimization efforts. However, I probably won't have time to add this feature to Scope Exit...
Ahhhh! The needed class is already part of scope exit. Just add a dismiss function, take it out of aux/detail and I WILL BE YOUR UNCLE, forget about Bob. Forget the optimization. Boost Scope.Exit doesn't use any of these tricks right? It's already using a boost function. Lorenzo my man, just add it. Sohail
On Sat, Sep 15, 2012 at 6:26 AM, Sohail Somani <sohail@taggedtype.net> wrote:
On 15/09/2012 2:16 AM, Lorenzo Caminiti wrote:
Lorenzo, thanks for volunteering but the design and implementation is far more complex than the extract from the doc. If we want it to be efficient, it shouldn't use boost.function, boost/multi_index/detail/scope_guard.hpp should be a good staring point.
I see. Then (speaking personally) it might make sense for it to exist in a lib as its impl is not trivial due to the optimization efforts. However, I probably won't have time to add this feature to Scope Exit...
Ahhhh!
The needed class is already part of scope exit.
What's there as boost::scope_exit::aux::guard is similar to what shown in the examples as scope_exit but it uses the operator= and it has a default constructor instead of using the constructor to initialize the functor. It is used to implement the Scope Exit macros using C++11 lambdas but only if !BOOST_NO_LAMBDA and BOOST_SCOPE_EXIT_CONFIG_USE_LAMBDAS--see the bottom of this subsection: http://www.boost.org/doc/libs/1_51_0/libs/scope_exit/doc/html/scope_exit/tut...
Just add a dismiss function,
I wouldn't add the dismiss functionality. That is not always needed with RAII and the Scope Exit API is more generic without it. If users need such functionality, they can use a commit/dismiss variable external to Scope Exit to simply implement it: struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { f_(); } private: std::function<void (void)> f_; }; bool commit = false; persons_.push_back(a_person); scope_exit on_exit1([&commit, this]() { if(!commit) persons_.pop_back(); });
take it out of aux/detail and I WILL BE YOUR UNCLE, forget about Bob.
Forget the optimization. Boost Scope.Exit doesn't use any of these tricks right? It's already using a boost function.
Lorenzo my man, just add it.
As I said I could but only if it's about adding something as simple as the above scope_exit... I'm not sure if that's enough. HTH, --Lorenzo
On 15/09/2012 1:55 PM, Lorenzo Caminiti wrote:
I wouldn't add the dismiss functionality. That is not always needed with RAII and the Scope Exit API is more generic without it. If users need such functionality, they can use a commit/dismiss variable external to Scope Exit to simply implement it:
Every scope guard since the beginning of time has had a dismiss/release function. Pardon me but writing an extra line is exactly what I'm trying to avoid. I could have written 35 such classes with all this email. Good thing I have a "libsohail" to stuff this into. Sohail
2012/9/16 Sohail Somani <sohail@taggedtype.net>:
On 15/09/2012 1:55 PM, Lorenzo Caminiti wrote:
I wouldn't add the dismiss functionality. That is not always needed with RAII and the Scope Exit API is more generic without it. If users need such functionality, they can use a commit/dismiss variable external to Scope Exit to simply implement it:
Every scope guard since the beginning of time has had a dismiss/release function.
Pardon me but writing an extra line is exactly what I'm trying to avoid.
Similarly, one might argue that every string class should have a trim member function, but then there are good arguments, that algorithms like trim should be external functions [1]. So, maybe the dismiss functionality can be an external function (assuming a guard can be rebound to another function): void do_nothing() {} void dismiss( guard& ) { guard = do_nothing; } Regards, Kris [1] http://www.gotw.ca/gotw/084.htm
On 12-09-16 9:35 AM, Krzysztof Czainski wrote:
Every scope guard since the beginning of time has had a dismiss/release function.
Pardon me but writing an extra line is exactly what I'm trying to avoid.
Similarly, one might argue that every string class should have a trim member function, but then there are good arguments, that algorithms like trim should be external functions [1].
So, maybe the dismiss functionality can be an external function (assuming a guard can be rebound to another function):
A valid argument if both methods use the same lines of code: s.trim() trim(s) VS: guard g(...); g.dismiss(); bool dismiss=false; guard g(...) dismiss=true; Sohail
On 12-09-16 9:35 AM, Krzysztof Czainski wrote:
So, maybe the dismiss functionality can be an external function (assuming a guard can be rebound to another function):
void do_nothing() {} void dismiss( guard& ) { guard = do_nothing; }
Oh hay, I see what you were saying here. This would be part of Scope Exit. Yeah this is OK. If a little weird :) Sorry, had a late night and the dog woke me up early. Sohail
On 12-09-16 9:35 AM, Krzysztof Czainski wrote:
So, maybe the dismiss functionality can be an external function (assuming a guard can be rebound to another function):
void do_nothing() {} void dismiss( guard& ) { guard = do_nothing; }
guard::operator=( guard const& ), if it exists, should probably execute the stored function before replacing it. To see why, consider: struct X { guard g_; FILE* f_; explicit X( FILE* f ): g_( fclose, f ), f_( f ) {} }; and think what happens in X::operator=( X const& ). dismiss() is different - it's the equivalent of auto_ptr::release (not of auto_ptr::op= or reset). And yes, I do see that the above example uses guard::op=( F f ), not the copy assignment, but it wouldn't be very consistent for the two to differ in such a fundamental way.
2012/9/16 Peter Dimov <lists@pdimov.com>:
On 12-09-16 9:35 AM, Krzysztof Czainski wrote:
So, maybe the dismiss functionality can be an external function (assuming a guard can be rebound to another function):
void do_nothing() {} void dismiss( guard& ) { guard = do_nothing; }
guard::operator=( guard const& ), if it exists, should probably execute the stored function before replacing it. To see why, consider:
struct X { guard g_; FILE* f_;
explicit X( FILE* f ): g_( fclose, f ), f_( f ) {} };
and think what happens in X::operator=( X const& ).
dismiss() is different - it's the equivalent of auto_ptr::release (not of auto_ptr::op= or reset).
And yes, I do see that the above example uses guard::op=( F f ), not the copy assignment, but it wouldn't be very consistent for the two to differ in such a fundamental way.
Right, I missed that. So dismiss(g) should not be implemented in terms of operator=, but it still can be an external function replacing the internal function with do_nothing. But maybe this point shows, that dismiss belongs inside class guard. Kris
On Sat, Sep 15, 2012 at 10:55 AM, Lorenzo Caminiti <lorcaminiti@gmail.com> wrote:
As I said I could but only if it's about adding something as simple as the above scope_exit... I'm not sure if that's enough.
Here's how it could look like. 1) As of now you can use SCOPE_EXIT on both C++03 and C++11 to do the following (macros :( ): void close_handle(int& h) { std::cout << "closing: " << h << std::endl; h = 0; } int main() { bool dismiss = false; int x = 1; BOOST_SCOPE_EXIT(&dismiss, &x) { if(!dismiss) close_handle(x); } BOOST_SCOPE_EXIT_END ... 2) If the following (non optimized) class where to be added to Scope Exit: namespace boost { namespace scope_exit { struct guard { template<typename Lambda> guard(Lambda f) : f_(f) {} ~guard() { f_(); } private: boost::function<void ()> f_; }; } } // namespace It could be used on C++11 to do the following (no macros :) but C++11 only :( ): int y = 2; boost::scope_exit::guard exit_y = [&dismiss, &y] { if(!dismiss) close_handle(y); }; 3) On C++03, if you were willing to write an extra close_handle_unless function (no macros :) but extra function :( , if you use a local function you are using macros again :( so option 1 is a better option): void close_handle_unless(bool const cond, int& h) { if(!cond) close_handle(h); } .... int z = 3; boost::scope_exit::guard exit_z = boost::bind( close_handle_unless, boost::ref(dismiss), boost::ref(z)); 4) On C++03, if you were willing to use Phoenix (no macro :( but extra Phoenix adapted function and Phoenix's syntax :( ): BOOST_PHOENIX_ADAPT_FUNCTION(void, close_handle_, close_handle, 1) ... int w = 4; boost::scope_exit::guard exit_w = if_(!ref(dismiss)) [ close_handle_(ref(w)) ] ; A complete example is attached. Also note how in this case the same dismiss control variable is used to dismiss all the scope exits--that's an argument to leave the dismiss feature up to the user and outside RAII lib (as if each scope exit guard had a dismiss data member, I would have to sync all of them one-by-one increasing the probability of a bug) in addition to the fact that in some cases there could be no need for the dismiss functionality but the RAII patter would still apply (IMO, the RAII patter and the dismiss functionality are separate things, they should be linked only by the user when that makes sense for the user's application logic). Personally, I'd just use the Scope Exit macros if I need to write code portable between C++03 and C++11 (option 1) and the (trivial) guard class if I have to write code that can rely on C++11 lambdas (option 2). HTH, --Lorenzo
Lorenzo Caminiti <lorcaminiti <at> gmail.com> writes:
Sure if there's enough interest I can move this scope_exit class from the examples into the Scope Exit lib itself when #ifndef BOOST_NO_LAMBDAS.
Lorenzo, are you the maintainer of Boost.ScopeExit? (Sorry that I didn't know that if so.)
My _personal_ opinion is that such a boost::scope_exit class is so trivial that programmers can just program it themselves, but that's just my opinion and I'm happy to add such a feature (or better to move it into the lib from the examples) if Boosters want it. No review should be needed, I can just post the (trivial) changes on this ML and confirm them with Alexander Nasonov.
That's not really the point. Making a library -- not just one class that uses std::tr1::function, but a library with other options will increase the use of RAII. It offers more elegant ways to do RAII and produces better code. And again, I'm talking about two classes here -- maybe more if others have ideas, but, I have two. The simplest by far is RAIIFunction which is very similar to your scope_exit example. The other is doesn't require std::tr1::function and is probably much more important to add to a library. I'm really not trying to knock what already exists, I just think we can do more. People could've argued that std::tr1::array wasn't needed, we had POD arrays and we have vectors, and they are a bit awkward when being passed to a function that wants to read the size but not have it encoded in an argument, but still they are a great addition to C++. Boost.ScopeExit has it's place, it probably even belongs in a general RAII 0library, but the library should have more than one tool. After all, I don't really need scoped_ptr, I can use shared_ptr for all of the same things, but me and many other programmers really like having both for various reasons. Does that make sense? As a reminder, regardless of final names, what I'm referring to here is: - RAIIFunction: Uses a member std::tr1::function that is executed in the destructor, etc. Works with bind and lambdas. - RAIIWrapper: Only works with functions taking one argument of the type of the resource. Is almost entirely static. Works great with C++03. Is like scoped_ptr but isn't just for pointers or things that look like pointers. Doesn't work with bind or lambda's that I know of, though it might happen to work with Apple's C "blocks". Thanks, Andrew Sandoval Thanks, Andrew Sandoval
On 14/09/2012 4:08 PM, Vicente J. Botet Escriba wrote:
Yes, I'm. Encapsulating the dismiss condition on the scope_guard doesn't scale when you have several actions that need to be executed. Compare
bool dismiss = false;
For me it is not very common to have to dismiss more than one. Maybe others can chime in. So for me, it would be beyond annoying to have to declare the bool myself because I'd have to do it every single time. So I'm back to writing my own class. I don't think you lose anything by doing this: struct scope_exit { scope_exit(std::function<void (void)> f) : f_(f) {} ~scope_exit(void) { if(f) f_(); } void dismiss(){ f_.swap(std::function<void(void)>()); } // OMG IF WE DO THIS THIS MEANS WE ARE USING JAVA private: std::function<void (void)> f_; };
Or in C++11:
boost::scope_guard lolwhat( [&](){ CloseHandle(handle); } );
... lolwhat.dismiss(); What about boost::exception_guard somename( [&](){ CloseHandle(handle); } );
14.09.2012 22:48, Sohail Somani пишет: that calls the function/lambda only if std::uncaught_exception() returns true? Another example: extern "C" int SomeFunc( char **Ret1, char **Ret2 ) { try { *Ret1 = new char[100]; boost::exception_guard Ret1Guard( [&](){ delete [] *Ret1; } ); *Ret2 = new char[100]; boost::exception_guard Ret2Guard( [&](){ delete [] *Ret2; } ); //try to fill the buffers with the data return 0; } catch( std::exception &ex ) { Log(ex); return -1; } } Note that dismisses are not required, and you even can use some macro magic to get rid of the meaningful guard variable names. -- Best regards, Sergey Cheban
On 14/09/12 19:33, Sergey Cheban wrote:
14.09.2012 22:48, Sohail Somani пишет:
Or in C++11:
boost::scope_guard lolwhat( [&](){ CloseHandle(handle); } );
... lolwhat.dismiss(); What about boost::exception_guard somename( [&](){ CloseHandle(handle); } ); that calls the function/lambda only if std::uncaught_exception() returns true?
std::uncaught_exception is very dangerous to use in this way. See http://www.gotw.ca/gotw/047.htm In short: your function will misbehave if called from a destructor during stack unwinding. John Bytheway
boost::scope_guard lolwhat( [&](){ CloseHandle(handle); } );
... lolwhat.dismiss();
What about boost::exception_guard somename( [&](){ CloseHandle(handle); } ); that calls the function/lambda only if std::uncaught_exception() returns true?
std::uncaught_exception is very dangerous to use in this way. See
http://www.gotw.ca/gotw/047.htm This article is not about my idea. I do not propose to throw from the destructor. I propose to use uncaught_exception() to determine whether
15.09.2012 5:27, John Bytheway пишет: the resources must be freed on the scope exit or not. I propose to use the following { HANDLE h = CheckHandle( CreateFile(...) ); //CheckHandle() may throw boost::exception_guard guard( [&](){ CloseHandle(h); } ); //other code that can throw return h; //note that h must not be closed } instead of the following { bool bDontClose = false; HANDLE h = CheckHandle( CreateFile(...) ); //CheckHandle() may throw boost::scope_guard guard( [&](){ if( !bDontClose ) CloseHandle(h); } ); //other code that can throw bDontClose = true; return h; //note that h must not be closed }
In short: your function will misbehave if called from a destructor during stack unwinding. It seems strange to me. Destructors are the only places where uncaught_exception() may return true, right? What's wrong with the following?
~exception_guard() { if( uncaught_exception() ) f(); } Is it less safe than the following? ~scope_guard() { f(); } -- Best regards, Sergey Cheban
On Sat, Sep 15, 2012 at 11:11 AM, Sergey Cheban <s.cheban@drweb.com> wrote:
15.09.2012 5:27, John Bytheway пишет: [snip
std::uncaught_exception is very dangerous to use in this way. See
This article is not about my idea. I do not propose to throw from the destructor. I propose to use uncaught_exception() to determine whether the resources must be freed on the scope exit or not.
It is not only about the detection if your code can throw or not, but about the fact that if you use your exception guard in a destructor, it will behave differently if the destructor is called during stack unwinding when an exception propagates and differently when the destructor is called under "normal" circumstances (i.e. no exception being thrown) see below:
I propose to use the following
{ HANDLE h = CheckHandle( CreateFile(...) ); //CheckHandle() may throw boost::exception_guard guard( [&](){ CloseHandle(h); } );
//other code that can throw
return h; //note that h must not be closed }
instead of the following
{ bool bDontClose = false; HANDLE h = CheckHandle( CreateFile(...) ); //CheckHandle() may throw boost::scope_guard guard( [&](){ if( !bDontClose ) CloseHandle(h); } );
//other code that can throw
bDontClose = true; return h; //note that h must not be closed
}
In short: your function will misbehave if called from a destructor during stack unwinding.
It seems strange to me. Destructors are the only places where uncaught_exception() may return true, right? What's wrong with the following?
~exception_guard() { if( uncaught_exception() ) f(); }
Is it less safe than the following?
~scope_guard() { f();
}
Whats wrong is (if I'm not teribly mistaken) is best illustrated by the "transaction" example in the GOTW: Transaction::~Transaction() { if( uncaught_exception() ) { RollBack(); } } If the transaction is used in a destructor: U::~U() { try { Transaction t( /*...*/ ); // do work } catch( ... ) { // clean up } } Then the transaction is *always* rolled back (even if it could complete successfully) when U::~U is called during exception propagation: { U u; // ... something_that_throws(); } // <- the transaction in U::~U is executed and always rolled back // regardless if it completes successfully Best, Matus
15.09.2012 13:36, Matus Chochlik пишет:
Whats wrong is (if I'm not teribly mistaken) is best illustrated by the "transaction" example in the GOTW:
Transaction::~Transaction() { if( uncaught_exception() ) { RollBack(); } }
If the transaction is used in a destructor:
U::~U() { try { Transaction t( /*...*/ ); // do work } catch( ... ) { // clean up } } Got it.
Things probably would be better with if( uncaught_exception_level() > m_UncaughtLevel ) { RollBack(); } but there is no such function in C++. :-( -- Best regards, Sergey Cheban
Lorenzo Caminiti <lorcaminiti <at> gmail.com> writes:
Please consider programming:
#define UNARY_SCOPE_EXIT(unary_func, arg) \ BOOST_SCOPE_EXIT( (&arg) ) { \ unary_func(arg); \ } BOOST_SCOPE_EXIT_END
And then:
UNARY_SCOPE_EXIT(CloseHandle, handle);
That way the code for this one specialized use case of Scope Exit becomes closer to what your expressed preference is. You can of course put this macro #definition into your own little lib scope_exit_arity.hpp and re-use it across your different projects :)
Am I the only one who doesn't like using MACROs -- especially multi-line macros, for things that can be done as well without? Maybe I'm too old, but it used to be the case that source debuggers really struggled with macros. You could step over the magic but not into it. -Andrew Sandoval
On 12.09.2012 05:40, Andrew Sandoval wrote:
The first of these is RAIIFunction (I'm open to other names and coding conventions).
This approach has one problem. The typical usage is something like that: FILE* f = fopen(...); if (!f) return; RAIIFunction<void> close_f = [f](){ fclose(f); } // may throw!!! The construction of std::function for RAIIFunction may throw std::bad_alloc. In that case you will get leaked file handle. That is why approach with scope capturing reference to a base class is better.
The other one I call just RAIIWrapper, and it works with static deleter functions taking a single argument of the type being delete.
This is somewhat similar to my library RCPP. http://rain.ifmo.ru/~sorokin/rcpp/
Ivan Sorokin <vanyacpp <at> gmail.com> writes:
On 12.09.2012 05:40, Andrew Sandoval wrote:
The first of these is RAIIFunction (I'm open to other names and coding conventions).
This approach has one problem.
The typical usage is something like that:
FILE* f = fopen(...); if (!f) return;
RAIIFunction<void> close_f = [f](){ fclose(f); } // may throw!!!
The construction of std::function for RAIIFunction may throw std::bad_alloc. In that case you will get leaked file handle.
Yes it could, but that is why I would always use the RAIIWrapper class for something as simple as fclose. e.g.: RAIIWrapper<RAIIDeleter(fclose)> pFile = fopen(...); if(!pFile.IsValid()) return; Because RAIIWrapper doesn't alloc anything, it will construct without risk of throwing. And because it has a built-in no-delete value, you don't need to check for NULL before letting it call fclose. It's a better solution for most things, but there are cases where RAIIFunction with a lambda or a function produced by bind are more desirably, and the user will need to understand the risks -- which in the large major of cases are next to nil anyway, before they elect to use RAIIFunction. (I'm not suggesting that an alloc failure isn't possible, I know it is, and I know that in a library you can't possibly anticipate all of the possible uses -- I am just suggesting that we have to deal with that kind of thing all of the time anyway. I don't know about you, but I use vector all of the time in place of malloc'd memory, because of it's RAII features, because it's expandable, etc. The risks are much the same. -Andrew Sandoval
On Friday 14 September 2012 21:25:44 Andrew Sandoval wrote:
It's a better solution for most things, but there are cases where RAIIFunction with a lambda or a function produced by bind are more desirably, and the user will need to understand the risks -- which in the large major of cases are next to nil anyway, before they elect to use RAIIFunction.
The probability of exception doesn't matter. Scope guards (or RAIIFunction) are typically used to implement exception safety, so introducing the possibility of exception from the scope guard itself sort of defeats its purpose. At the very least, it will complicate usage - probably, more than it will simplify one. IMHO, this tool should not be based on boost/std::function.
Andrey Semashev <andrey.semashev <at> gmail.com> writes:
The probability of exception doesn't matter. Scope guards (or RAIIFunction) are typically used to implement exception safety, so introducing the possibility of exception from the scope guard itself sort of defeats its purpose. At the very least, it will complicate usage - probably, more than it will simplify one. IMHO, this tool should not be based on boost/std::function.
I knew someone would say that about the possibility of exceptions. Again, I would argue that the other class doesn't have that problem and is a better fit for most things, but I don't see anyway your going to get std::tr1::function functionality without allocations and the risk of an exception -- unless it is implemented so that an exception can't be thrown. I disagree completely about it defeating the purpose. You can find something less than perfect in just about every library, but that doesn't mean they aren't tremendously useful. Again though, a collection of RAII wrapper classes is what I'm proposing. And the great thing about having them in Boost, besides the fact that I don't have to re-write them over and over, is that all of the caveats can be well documented, as they are in other libraries. Are we making any progress here? I don't think I'm alone in seeing the value of RAII classes and promoting the practice. Thanks. -Andrew Sandoval
On Saturday 15 September 2012 01:13:51 Andrew Sandoval wrote:
Andrey Semashev <andrey.semashev <at> gmail.com> writes:
The probability of exception doesn't matter. Scope guards (or RAIIFunction) are typically used to implement exception safety, so introducing the possibility of exception from the scope guard itself sort of defeats its purpose. At the very least, it will complicate usage - probably, more than it will simplify one. IMHO, this tool should not be based on boost/std::function. I knew someone would say that about the possibility of exceptions. Again, I would argue that the other class doesn't have that problem and is a better fit for most things, but I don't see anyway your going to get std::tr1::function functionality without allocations and the risk of an exception -- unless it is implemented so that an exception can't be thrown.
There are several alternatives to std::function. One has been pointed out in this thread several times - capture the scope guard by reference to base. This is my preferred solution. I can also suggest another option: use fixed sized buffer to place the function object on the stack. The size of the buffer can be specified as a template parameter of the scope guard. This gives you runtime flexibility (which I don't think is necessary) but it makes the code less portable. I personally don't like it, unless you can provide a motivating example where initialization of an already constructed scope guard is required.
I disagree completely about it defeating the purpose. You can find something less than perfect in just about every library, but that doesn't mean they aren't tremendously useful.
It's not just imperfection. It simply disables the main use case of the tool, unless you close your eyes an put a comment "I feel lucky" before the scope guard. If your function-based scope guard is not intended to be used for exception safety then define its purpose. And don't call it RAII because it can't be used for resource management.
Again though, a collection of RAII wrapper classes is what I'm proposing. And the great thing about having them in Boost, besides the fact that I don't have to re-write them over and over, is that all of the caveats can be well documented, as they are in other libraries.
Are we making any progress here? I don't think I'm alone in seeing the value of RAII classes and promoting the practice.
I'm not arguing with that.
participants (13)
-
Alexander Nasonov -
Andrew Sandoval -
Andrey Semashev -
Ivan Sorokin -
John Bytheway -
Klaim - Joël Lamotte -
Krzysztof Czainski -
Lorenzo Caminiti -
Matus Chochlik -
Peter Dimov -
Sergey Cheban -
Sohail Somani -
Vicente J. Botet Escriba