Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
On 07/19/2012 11:03 PM, julien2412 wrote: Thank you David for the hint! I re read the An in-depth explanation part of http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550, it's great ! I understood when and above all how the memory is cleaned. About your multidimalgorithm, what would be the problem if it'd be changed like this ? (to sum up: passing parameter by value to optimize) It's a well-known but (in my impression) not-too-widely used idiom. And IIRC it even has no drawbacks (which is sometimes hard to tell, C++ being so baroque and fragile that something that looks rather innocuous can actually be an epic catastrophe). Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Thank you David for the hint!
I re read the An in-depth explanation part of
http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550,
it's great ! I understood when and above all how the memory is cleaned.
About your multidimalgorithm, what would be the problem if it'd be changed
like this ? (to sum up: passing parameter by value to optimize)
/include/mdds/flat_segment_tree.hpp
flat_segment_treekey_type, value_type
operator=(flat_segment_treekey_type, value_type other);
/include/mdds/flat_segment_tree_def.inl
templatetypename _Key, typename _Value
flat_segment_tree_Key, _Value
flat_segment_tree_Key, _Value::operator=(flat_segment_tree_Key,
_Value
other)
{
swap(copy);
return *this;
}
this part of
http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550
made me think about this:
Observe that upon entering the function that all the new data is already
allocated, copied, and ready to be used. This is what gives us a strong
exception guarantee for free: we won't even enter the function if
construction of the copy fails, and it's therefore not possible to alter the
state of *this. (What we did manually before for a strong exception
guarantee, the compiler is doing for us now; how kind.)
Is this design pattern without flaw (does it exist ? :-)) and be applied in
every case in LO code ?
Thank you too Stephan about the refs. First I must finish Stroustrup's book
and the way is long because I never really coded in C++ just some Java (when
it was in 1.3) and few C and VB.
Julien
--
View this message in context:
http://nabble.documentfoundation.org/PATCH-cleaning-before-assignment-lotuswordpro-source-filter-xfilter-xfparastyle-cxx-tp3995480p3996515.html
Sent from the Dev mailing list archive at Nabble.com.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Hi,
On Thu, Jul 19, 2012 at 02:03:39PM -0700, julien2412 wrote:
Thank you David for the hint!
I re read the An in-depth explanation part of
http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550,
it's great ! I understood when and above all how the memory is cleaned.
About your multidimalgorithm, what would be the problem if it'd be changed
like this ? (to sum up: passing parameter by value to optimize)
/include/mdds/flat_segment_tree.hpp
flat_segment_treekey_type, value_type
operator=(flat_segment_treekey_type, value_type other);
/include/mdds/flat_segment_tree_def.inl
templatetypename _Key, typename _Value
flat_segment_tree_Key, _Value
flat_segment_tree_Key, _Value::operator=(flat_segment_tree_Key,
_Value
other)
{
swap(copy);
return *this;
}
It is equivalent to the current code. Just most C++ programmers
(including me :-) are accustomed to the fact that operator= takes its
argument by const reference...
this part of
http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550
made me think about this:
Observe that upon entering the function that all the new data is already
allocated, copied, and ready to be used. This is what gives us a strong
exception guarantee for free: we won't even enter the function if
construction of the copy fails, and it's therefore not possible to alter the
state of *this. (What we did manually before for a strong exception
guarantee, the compiler is doing for us now; how kind.)
Yeah, it is even a line shorter :-) But whether a function call is (not)
eliminated in the case an exception is thrown would be the last of my
worries (it is _exceptional_ situation, after all... And the gain of
avoiding the call would be more than offset by the cost of the exception
processing mechanism.)
Is this design pattern without flaw (does it exist ? :-)) and be applied in
every case in LO code ?
Well, we do not typically care so much about exceptions in LO (heck,
there is code--and probably a lot of it--that does not even have weak
exception guarantee). So it is probably not so useful to start adapting
the assignment operators in our codebase en masse.
D.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
On 07/17/2012 11:52 PM, julien2412 wrote: To sum up, it's very interesting but clearly it's too complicated for me. If you are interested in mastering C++, stuff like this (and much more) is well covered in the seminal books by Sutter (Exceptional C++, More Exceptional C++, etc.) and Meyers (Effective C++, More Effective C++, etc.). Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Thank you for the links David. I understand this design : - avoids duplication - guarantees that the new object can be completely built before swapping from old to new one But I still don't know at which moment do we delete old resources ? Or more precisely, it seems it doesn't remove anything so how can't it be a leak ? Other things I read : 1) The one thing to be careful of is to make sure that the swap method is a true swap, and not the default std::swap which uses the copy constructor and assignment operator itself. Typically a memberwise swap is used. std::swap works and is 'no-throw' guaranteed with all basic types and pointer types. Most smart pointers can also be swapped with a no-throw guarantee. (from http://stackoverflow.com/questions/1734628/copy-constructor-and-operator-overload-in-c-is-a-common-function-possible/1734640#1734640 ) 2) All the subtleties from http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550 which ends with other things again to take into account C++11 To sum up, it's very interesting but clearly it's too complicated for me. Perhaps by reading it several times, I'll fully understand it (let's be optimistic :-) ) Julien -- View this message in context: http://nabble.documentfoundation.org/PATCH-cleaning-before-assignment-lotuswordpro-source-filter-xfilter-xfparastyle-cxx-tp3995480p3996129.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Hi, On Tue, Jul 17, 2012 at 02:52:22PM -0700, julien2412 wrote: Thank you for the links David. I understand this design : - avoids duplication - guarantees that the new object can be completely built before swapping from old to new one But I still don't know at which moment do we delete old resources ? Or more precisely, it seems it doesn't remove anything so how can't it be a leak ? It is not a leak :-) The old resources are deleted by destructor (avoiding duplication, right?) when the copy (that now contains the old content of this) goes out of scope. D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
On Sat, Jul 14, 2012 at 07:04:07AM -0700, julien2412 wrote:
Hello,
Keeping on slowly reading Bjarne Stroustrup's C++ book, I read that for =
operator functions, we must delete/free the members before = assignment
(which must copy).
Yes. Additionally, we must guard against self-assignment, because it
would lead to use of already deleted object. So, the canonical way to
write operator= is:
Foo operator=(Foo const other)
{
if (this != other)
{
// copy
}
return *this;
}
Or, if the class implements swap (and has accessible copy constructor,
which it should always have when it has operator=):
Foo operator=(Foo const other)
{
Foo copy(other);
swap(copy);
return *this;
}
D.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Thank you David for your answer. I updated the way to check self assignment and add the cleaning of members (see http://cgit.freedesktop.org/libreoffice/core/commit/?id=00ae1cec08ae970f0640c0cc5d612eb0b9035207) About the swap part, if you've got an example, I'd be interested. Julien -- View this message in context: http://nabble.documentfoundation.org/PATCH-cleaning-before-assignment-lotuswordpro-source-filter-xfilter-xfparastyle-cxx-tp3995480p3995919.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Hi, On Mon, Jul 16, 2012 at 01:57:43PM -0700, julien2412 wrote: Thank you David for your answer. I updated the way to check self assignment and add the cleaning of members (see http://cgit.freedesktop.org/libreoffice/core/commit/?id=00ae1cec08ae970f0640c0cc5d612eb0b9035207) Heh, it already did self assignment check, just with early return... You do not have to take my suggestions literally :-) About the swap part, if you've got an example, I'd be interested. Okay, you can look at http://code.google.com/p/multidimalgorithm/source/detail?spec=svn0016d50d9cf6c6437598d196f59b2aa52110c8f9r=bb3ea2226e49540027df50072aef4c86188f7d64# Here is a detailed explanation (the example uses friend swap function instead of member function, but that is just a detail): http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550#3279550 D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[PATCH] cleaning before = assignment lotuswordpro/source/filter/xfilter/xfparastyle.cxx
Hello, Keeping on slowly reading Bjarne Stroustrup's C++ book, I read that for = operator functions, we must delete/free the members before = assignment (which must copy). So I attach a straightforward patch for lotuswordpro/source/filter/xfilter/xfparastyle.cxx. http://nabble.documentfoundation.org/file/n3995480/patch_lotuswordpro.txt patch_lotuswordpro.txt Before I commit and push it on master, I would like to know first if i'm wrong or not about all this. Perhaps I'm wrong or perhaps it's useless because of a C++ mechanism I don't know or missed. Julien -- View this message in context: http://nabble.documentfoundation.org/PATCH-cleaning-before-assignment-lotuswordpro-source-filter-xfilter-xfparastyle-cxx-tp3995480.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
