From bc3404fcbea75b02531d4da2373ade66eb7c5985 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Sun, 12 Nov 2017 23:42:58 +0100 Subject: [PATCH] SGSharedPtr: more efficient copy and move assignment operators The copy-and-swap idiom is certainly very cute, but often causes unnecessary copies. My commit fedafb9352c2877c5983e90834ffba0fff80ff15 did exactly that, unfortunately. Restore the exact same code for the copy-assignment operator as before commit fedafb935, and add a more efficient implementation for the move-assignment operator. As explained by Howard Hinnant in [1] and [2], if some particular piece of code really needs a strong exception safety guarantee, one can easily add a specific method for that; this is not a valid reason to make the code slower for all other places that have no use for such a guarantee! [1] http://www.slideshare.net/ripplelabs/howard-hinnant-accu2014 [2] https://stackoverflow.com/a/9322542/4756009 --- simgear/structure/SGSharedPtr.hxx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/simgear/structure/SGSharedPtr.hxx b/simgear/structure/SGSharedPtr.hxx index 181dcbb6..ce242f05 100644 --- a/simgear/structure/SGSharedPtr.hxx +++ b/simgear/structure/SGSharedPtr.hxx @@ -67,10 +67,19 @@ public: ~SGSharedPtr(void) { reset(); } - // This handles copy assignment using the copy-and-swap idiom, and also move - // assignment thanks to the move construtor. - SGSharedPtr& operator=(SGSharedPtr p) - { swap(p); return *this; } + SGSharedPtr& operator=(const SGSharedPtr& p) + { reset(p.get()); return *this; } + + SGSharedPtr& operator=(SGSharedPtr&& p) + { // Whether self-move is to be supported at all is controversial, let's + // take the conservative approach for now + if (this != &p) { + swap(p); + p.reset(); + } + return *this; + } + template SGSharedPtr& operator=(const SGSharedPtr& p) { reset(p.get()); return *this; }