GNOME Bugzilla – Bug 486373
mention visit_each in adapts documentation
Last modified: 2014-07-27 14:51:43 UTC
I have implemented my own adaptor based on example included in sigc::adapts documentation. Everything was fine but today I have discovered that slots are not invalidated when they should be (because of sigc::trackable stuff). After few hours of intensive analyzing sigc++ code I've found that my implementation does not provide visit_each specialization and it probably should. Could you, please, mention visit_each somewhere near that "my_adpator" example? I think it is rather important because code compiles without and some nasty segfaults can happen if slots are not invalidated.
(Sorry for the very late response.) You understand this, so you would be the best person to write that documentation. A patch would be ideal. I don't use that part of the API myself.
Sorry, but I've forgotten a lot of the original context and I'm to lazy to do it properly, so I'll just put it here ;-) If you implement your own adaptor, you must also provide your specialization of sigc::visit_each that will forward the call to the functor(s) your adapter is wrapping. This is needed because of the way C++ works. Otherwise bad things may happen. For example pointers stored within the functor won't be invalidated when sigc::trackable object is destroyed and you can end up executing call backs on destroyed objects. Here is a simple example: template <class T_functor> struct my_adaptor : public sigc::adapts<T_functor> { explicit my_adaptor(const T_functor& _a_functor) : sigc::adapts<T_functor>(_a_functor) {} ... }; template <class T_action, class T_functor> void visit_each(const T_action& _A_action, const my_adaptor<T_functor>& _A_target) { visit_each(_A_action, _A_target.functor_); } Note that this must be in the same namespace.
Created attachment 190208 [details] [review] patch: Mention visit_each() in the documentation of sigc::adapts I've made a git patch out of the info in comment 2. I've also made some other small changes to the code example in the documentation of sigc::adapts. It did not look quite right. For example, the name of the example struct was misspelt, and the constructor did not have the same name as the struct. The code example gives the impression of having been copied from struct adaptor_functor, and then not enough changed. I'm not sure all details are correct even after my changes, but I'm pretty sure it's at least better. It would be fine, if someone more familiar with libsigc++ than me could check the changed and added comments. If that's not possible within a reasonable time, I suggest the patch is still pushed. Only comments are changed.
Shall I push the patch?
Review of attachment 190208 [details] [review]: Yes, please commit, with the small change I've suggested here. ::: sigc++/adaptors/macros/adaptor_trait.h.m4 @@ -267,2 +274,5 @@ * @endcode * + * If you implement your own adaptor, you must also provide your specialization + * of visit_each<>() that will forward the call to the functor(s) your + * adapter is wrapping. This is needed because of the way C++ works. "This is needed because of the way C++ works." is rather free of information. "Otherwise bad things may happen. For example" I would change this to "Otherwise, ".
Committed with the suggested change: http://git.gnome.org/browse/libsigc++2/commit/?id=0439751f40e126918e5fa0f36415930b315dd7c8
The way visit_each() is handled has been changed. The change breaks API for those users of libsigc++ who have made their own adaptors, and corresponding visit_each() overloads. See bug 724496 and https://mail.gnome.org/archives/libsigc-list/2014-July/msg00003.html