GNOME Bugzilla – Bug 586436
Accumulator result_type cannot be different from slot return type
Last modified: 2010-12-23 21:46:53 UTC
Please describe the problem: I cannot define an accumulator whose result_type is different than the return type of the slots it evaluates. This makes it impossible to, for example, store all the return values in a vector. It all boils down to slot_iterator_buf being defined incorrectly. slot_iterator_buf is templated as: template <class T_emitter, class T_result = typename T_emitter::result_type> slot_iterator_buf But this is completely bogus. The result of the iterator is not supposed to be the result type of the emitter - it is supposed to be the result type of the slot! The emitter's result_type is the result_type defined in the accumulator. The iterators should return the result from the slots. The template declaration has to be changed to: template <class T_emitter, class T_result = typename T_emitter::slot_return_type> slot_iterator_buf and this typedef needs to be added to each signal_emitN: typedef T_return slot_return_type; This bug makes accumulators much less useful than they could be. I hope that this can be fixed without an ABI break, otherwise we are waist deep in FUBAR. Steps to reproduce: Try code like this: #include <sigc++/sigc++.h> #include <vector> template<class T> struct VectorAccumulator { typedef std::vector<T> result_type; template<class T_iterator> operator()(T_iterator first, T_iterator last) { result_type vec; for (; first != last; ++first) vec.push_back(static_cast<int>(*first)); return vec; } }; double square(double x) { return x*x; } double noop(double x) { return x; } int main() { sigc::signal<double, double>::accumulated<VectorAccumulator<int> > sig; sig.connect(sigc::ptr_fun(&square)); sig.connect(sigc::ptr_fun(&noop)); std::vector<int> v = sig.emit(3.0); for (std::vector<int>::iterator i = v.begin(); i != v.end(); ++i) { std::cout << *i << " "; } std::cout << std::endl; return 0; } Actual results: Does not compile. Expected results: The program compiles and prints "9 3 " when run. Does this happen every time? Yes Other information:
Ha, it seems it was already reported as bug #343967... The fix posted there requires less work.
Created attachment 137047 [details] [review] Fix and re-added test This fixes accumulators with a result type different from the return type of the slot. Additionally, it adds a test of a vector accumulator to test_accumulated.cc - previously this test only tested an accumulator where the return type of the slot and the result type of the accumulator were implicitly convertible between each other, so this bug was not caught. I also needed to add two autoconf macros, to only build this test on G++ - an AIX compiler was reported to be broken. I have no idea how to detect that broken compiler, so I'm being overly conservative and only running this test on the known-good G++.
Thanks, and sorry for not noticing this until now. I'd like some discussion of whether this is an ABI break, or even an API break. Also, we generally prefer to check for specific compiler problems with a test case, instead of hard-coding knowledge of particular compilers (and their behaviour at the time of coding). There are some existing configure tests. If you don't know what actually breaks, or don't have access to that compiler, just leave that for later, maybe adding a comment that you expect it to break on a certain compiler and that a configure test is needed.
The bug was in header-only code that is not included in the library at all, so it should be neither an API nor an ABI break. I don't have access to either AIX or Tru64. If breaking 'make check' on them is unacceptable, you can leave out the test. If ensuring that accumulators work as intended is more important, the test can be run on all compilers (maybe they are already fixed).
But will this change the ABI of any API (such as gtkmm) that uses libsigc++ signals? Could you please update the patch to avoid the compiler-specific (instead of behaviour-specific) check as suggested. The test case should stay.
Created attachment 150384 [details] [review] Fix v2, without configure cruft Cleaned up patch without the configure cruft.
This should not affect gtkmm or any other application that uses libsigc++. The only change is to explicitly specify the return type used by slot iterator buffers in one place, rather than using default values. For normal signals, this change does nothing, because the type that gets used is the same that was used before the change. The only impact of this patch is that code where signal return type != slot return type will now compile.
OK. Please commit. Thanks.
I think I don't have write access to the repository.
Committed with this ChangeLog entry. Please patch the ChangeLog in future. Many thanks for this fix and for reminding me about it. This will appear in a tarball release soon. 2009-12-29 Krzysztof Kosiński <tweenk.pl@gmail.com> Accumulators: Allow return types that are different to the signal's. * sigc++/macros/signal.h.m4: signal_emit*: Correct the slot_iterator_buf_type and slot_reverse_iterator_buf_type typedefs to allow accumulators with return types that are different to the signal's return type. * tests/Makefile.am: Reenable test_accumulated, so we can test the fix. It should be manually disabled if building on AIX (if the AIX problem cannot be fixed properly). * tests/test_accumulated.cc: Add an accumulator with a return type that is different to the signal's return type. In this case it's a vector listing all results. Bug #586436.
*** Bug 343967 has been marked as a duplicate of this bug. ***