Re:[boost] Re: Seriialzation library draft # 18 uploaded to files section
Joaquin Munoz wrote:
I think I detected a minor bug in collections_load_imp.hpp. The operator() code for archive_input_seq reads:
inline void operator()(Archive &ar, Container &s) { typedef BOOST_DEDUCED_TYPENAME Container::value_type type; stack_allocate<type> t; load_construct_data(ar, t.address(), 0U); ar >> make_nvp("item", t.reference()); s.push_back(t.reference()); t.address()->~type(); // undo load_construct_data above }
This is AFAICS not exception-safe: if say s.push_back() throws, the dtor for the stack-allocated variable t won't be called. The same problem in similar code snippets through this file. Apologies if my perception is wrong.
Hmmm - An interesting point I never considered. Push_back from the standard library has a strong guarantee so the problem should boil down to one of the constructors (copy or inplace) throwing. Try/catch block could be added but it could be a little expensive for large collections and would be unnecessary in the most common case where inplace and copy constructors don't throw. I would like to use has_nothrow_constructor and has_trhow copy to address this but the language in the type_traits document suggests that this wouldn't be effective. Anyone is free to chime in here. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
Hmmm - An interesting point I never considered. Push_back from the standard library has a strong guarantee so the problem should boil down to one of the constructors (copy or inplace) throwing. Try/catch block could be added but it could be a little expensive for large collections
I wouldn't worry about it until you've measured.
and would be unnecessary in the most common case where inplace and copy constructors don't throw. I would like to use has_nothrow_constructor and has_trhow copy to address this but the language in the type_traits document suggests that this wouldn't be effective. Anyone is free to chime in here.
There are other reasons to avoid try/catch. See http://www.boost.org/more/error_handling.html. You should give stack_allocate an internal flag that says when to destroy. Better yet, it seems, use a real variant or optional type as has been suggested, and if the libraries don't have the support you need, get their authors to add it. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Robert Ramey ha escrito:
Joaquin Munoz wrote:
I think I detected a minor bug in collections_load_imp.hpp. The operator() code for archive_input_seq reads:
inline void operator()(Archive &ar, Container &s) { typedef BOOST_DEDUCED_TYPENAME Container::value_type type; stack_allocate<type> t; load_construct_data(ar, t.address(), 0U); ar >> make_nvp("item", t.reference()); s.push_back(t.reference()); t.address()->~type(); // undo load_construct_data above }
This is AFAICS not exception-safe: if say s.push_back() throws, the dtor for the stack-allocated variable t won't be called. The same problem in similar code snippets through this file. Apologies if my perception is wrong.
Hmmm - An interesting point I never considered. Push_back from the standard library has a strong guarantee so the problem should boil down to one of the constructors (copy or inplace) throwing.
push_back() can also throw from the allocator if it runs out of memory, so you really need some proper clenaup regardless of the guarantees made by copying ops. Dave's suggestion of using RAII is probably the most elegant way to deal with the situation: in my experience, however, this sort of scope guards perform worse than a try{}catch(...){}. Your mileage may vary. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz <joaquin@tid.es> writes:
Robert Ramey ha escrito:
Joaquin Munoz wrote:
I think I detected a minor bug in collections_load_imp.hpp. The operator() code for archive_input_seq reads:
inline void operator()(Archive &ar, Container &s) { typedef BOOST_DEDUCED_TYPENAME Container::value_type type; stack_allocate<type> t; load_construct_data(ar, t.address(), 0U); ar >> make_nvp("item", t.reference()); s.push_back(t.reference()); t.address()->~type(); // undo load_construct_data above }
This is AFAICS not exception-safe: if say s.push_back() throws, the dtor for the stack-allocated variable t won't be called. The same problem in similar code snippets through this file. Apologies if my perception is wrong.
Hmmm - An interesting point I never considered. Push_back from the standard library has a strong guarantee so the problem should boil down to one of the constructors (copy or inplace) throwing.
push_back() can also throw from the allocator if it runs out of memory, so you really need some proper clenaup regardless of the guarantees made by copying ops. Dave's suggestion of using RAII is probably the most elegant way to deal with the situation: in my experience, however, this sort of scope guards perform worse than a try{}catch(...){}.
On a high-quality table-based EH implementation, that's often true. That said, there are other reasons to use RAII as I pointed out. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
participants (3)
-
David Abrahams -
Joaquín Mª López Muñoz -
Robert Ramey