GNOME Bugzilla – Bug 752560
Replace sigc::is_base_and_derived with std::is_base_of
Last modified: 2015-08-14 10:05:12 UTC
This patch replaces (and removes) sigc::is_base_and_derived with std::is_base_of, now that we can use C++11. However one of the tests now fails: $ ./tests/test_functor_trait Test 2 Expected "hit all ints: int: 2 " Got "hit all ints: "
Created attachment 307656 [details] [review] 0001-C-11-Use-std-is_base_of-instead-of-our-sigc-is_base_.patch
There is a subtle difference between sigc::is_base_and_derived and std::is_base_of, a difference that I had not noticed before. sigc::is_base_and_derived<T, T>::value is true for all T, even for T=int, due to the specialization template <class T_base> struct is_base_and_derived<T_base, T_base> { static const bool value = true; }; http://en.cppreference.com/w/cpp/types/is_base_of says Although no class is its own base, std::is_base_of<T, T>::value is true because the intent of the trait is to model the "is-a" relationship, and T is a T. Despite that, std::is_base_of<int, int>::value is false because only classes participate in the relationship that this trait models. sigc::is_base_and_derived<int, int>::value is true but std::is_base_of<int, int>::value is false. In most places in libsigc++ this difference is unimportant, because the first template parameter is a class or struct (sigc::functor_base, sigc::adaptor_base or sigc::trackable). The exception is sigc::visit_each_type<>() in visit_each.h. I don't know how to handle this. I'll think about it.
It's actually quite easy. In visit_each.h, change with_type<std::is_base_of<T_target, T_type>::value, T_type, T_self>::execute_(_A_type, *this); to with_type<std::is_base_of<T_target, T_type>::value || std::is_same<T_target, T_type>::value, T_type, T_self>::execute_(_A_type, *this); and corresponding change in the call to with_type_pointer<>::execute_(). You should also add #include <type_traits> in functor_trait.h.m4 and in visit_each.h.
Thanks. I've pushed that as an additional commit. I do wonder how we can use || on a typename, but I've seen that before so something makes it possible. It's nice to remove some code from libsigc++. I wonder if there's anything else we can replace.
> I do wonder how we can use || on a typename, but I've seen that before so > something makes it possible. We can't. It's not a typename. It's a bool value. Some template parameters are compile-time constant values. I've pushed a commit with the rest of the additions I recommended in comment 3. glibmm/tools/test_scripts/testheaders.sh can check if each header file includes all other header files that it depends on. > I wonder if there's anything else we can replace. I'm sure the code can be shrunk and most or all m4 files deleted, if we use variadic templates.
> I wonder if there's anything else we can replace. There are some unnecessary header files, some of them generated from .m4 files: sigc++/class_slot.h, class_slot.h.m4 sigc++/hide.h, hide.h.m4 sigc++/method_slot.h, method_slot.h.m4 sigc++/object.h sigc++/object_slot.h, object_slot.h.m4 sigc++/slot.h is not completely useless, but there's no reason to generate it from an m4 file. Is it too harsh to delete these header files now? It will break apps that happen to include them, and the gain in libsigc++ is insignificant. At least I can delete the m4 files, add a TODO comment in the header files, and store them in git.
API, bur not ABI, changes are fine. So we don't need to keep the headers. Avoiding generation is also nice. Thanks.
More useless files: sigc++/retype.h, retype.h.m4 The files sigc++/bind.h -> sigc++/adaptors/bind.h sigc++/bind_return.h -> sigc++/adaptors/bind_return.h sigc++/retype_return.h -> sigc++/adaptors/retype_return.h sigc++/slot.h -> sigc++/functors/slot.h only contain an #include of another header file. They are unnecessary, but I wonder if it's ok to delete all of them. Deleting sigc++/slot.h (and possibly sigc++/bind.h) is likely to generate protests. sigc++/bind.h is mentioned in the description of sigc++/sigc++.h as an example of a header file that can be included individually. sigc++/slot.h is included from files in glibmm and cairomm, and perhaps from other packages. I can fix glibmm, if we decide to delete sigc++/slot.h, but cairomm is stored in a git repository where I have no account.
> Deleting sigc++/slot.h is likely to generate protests. See bug 521418.
> There are some unnecessary header files, some of them generated from .m4 files: > > sigc++/class_slot.h, class_slot.h.m4 > sigc++/hide.h, hide.h.m4 > sigc++/method_slot.h, method_slot.h.m4 > sigc++/object.h > sigc++/object_slot.h, object_slot.h.m4 I have removed sigc++/class_slot.h, method_slot.h, and object_slot.h: https://git.gnome.org/browse/libsigc++2/commit/?id=9db2c413d20d170196e1de62884d4fb9b3f49578 https://git.gnome.org/browse/libsigc++2/commit/?id=f6372ffec592f3878961a2f3c34fe284b66872e3 https://git.gnome.org/browse/libsigc++2/commit/?id=88e88c8d6803a07dccbe2a603926f462520ff974 > More useless files: > sigc++/retype.h, retype.h.m4 I removed that sigc++/retype.h, which involved removing some unused m4 code: https://git.gnome.org/browse/libsigc++2/commit/?id=0b8d2f769deec397d5441c6429c4a2544d9f1a95 (In reply to Kjell Ahlstedt from comment #8) [snip] > The files > sigc++/bind.h -> sigc++/adaptors/bind.h > sigc++/bind_return.h -> sigc++/adaptors/bind_return.h > sigc++/retype_return.h -> sigc++/adaptors/retype_return.h > sigc++/slot.h -> sigc++/functors/slot.h > only contain an #include of another header file. They are unnecessary, but I > wonder if it's ok to delete all of them. [snip] They seem to be convenience headers, for instance so we can include sigc++/bind.h instead of sigc++/adaptors/bind.h. I don't see a particular reason to change that. But I agree that they shouldn't be generated. sigc++/hide.h looked like a convenience, but it didn't include adaptor/hide.h, so I removed that too: https://git.gnome.org/browse/libsigc++2/commit/?id=63eb3c4b86af3887a1cee6cf2efeca60e00fa0b6
I have stored sigc++/slot.h in git and removed sigc++/macros/slot.h.m4. https://git.gnome.org/browse/libsigc++2/commit/?id=9313889bd596afcd9a788061388b26c15d21351c I let the copyright year in sigc++/slot.h be 2002, because it was 2002 in the removed m4 file. Don't know if it's right or wrong or doesn't matter. Now it only remains to decide if sigc++/object.h shall be removed. There is no sigc::object class or function. That file only includes sigc++/trackable.h. I vote for removing it.
Agreed. Please go ahead. Thanks.
I have removed sigc++/object.h: https://git.gnome.org/browse/libsigc++2/commit/?id=2b80befa1033cb99d328c149a0535cc3d40c5109
> I'm sure the code can be shrunk and most or all m4 files deleted, if we use > variadic templates. I'm exploring that in bug #753612 .