[SIGNAL/BIND] Regressions with GCC 4.1 RC.
We se some strange problems with GCC 4.1 (the prerelease/release candidate) with use of boost::signal and boost::bind. Have anyone run the regressions tests for this GCC version? g++ (GCC) 4.1.0 20060219 (prerelease We get errors of this form: g++ -DHAVE_CONFIG_H -I. -I. -I../../src -I./.. -I../../boost -Wextra -Wall -I/usr/X11R6/include -g -O -MT render_graphic.lo -MD -MP -MF .deps/render_graphic.Tpo -c render_graphic.C -o render_graphic.o In file included from ../../boost/boost/config.hpp:35, from ../../boost/boost/bind.hpp:23, from ../../src/support/translator.h:16, from ../../src/graphics/GraphicsTypes.h:18, from ../../src/graphics/GraphicsLoader.h:27, from render_graphic.h:17, from render_graphic.C:13: ../../boost/boost/config/compiler/gcc.hpp:92:7: warning: #warning "Unknown compiler version - please run the configure tests and report the results" ../../boost/boost/bind.hpp: In function 'void boost::visit_each(V&, const boost::_bi::value<T>&, int) [with V = boost::signals::detail::bound_objects_visitor, T = const InsetBase*]': ../../boost/boost/bind.hpp:296: instantiated from 'void boost::_bi::list2<A1, A2>::accept(V&) const [with V = boost::signals::detail::bound_objects_visitor, A1 = boost::reference_wrapper<const LyX>, A2 = boost::_bi::value<const InsetBase*>]' ../../boost/boost/bind/bind_template.hpp:150: instantiated from 'void boost::_bi::bind_t<R, F, L>::accept(V&) const [with V = boost::signals::detail::bound_objects_visitor, R = const Buffer* const, F = boost::_mfi::cmf1<const Buffer* const, LyX, const InsetBase*>, L = boost::_bi::list2<boost::reference_wrapper<const LyX>, boost::_bi::value<const InsetBase*> >]' ../../boost/boost/bind.hpp:1110: instantiated from 'void boost::visit_each(V&, const boost::_bi::bind_t<R, F, L>&, int) [with V = boost::signals::detail::bound_objects_visitor, R = const Buffer* const, F = boost::_mfi::cmf1<const Buffer* const, LyX, const InsetBase*>, L = boost::_bi::list2<boost::reference_wrapper<const LyX>, boost::_bi::value<const InsetBase*> >]' ../../boost/boost/visit_each.hpp:25: instantiated from 'void boost::visit_each(Visitor&, const T&) [with Visitor = boost::signals::detail::bound_objects_visitor, T = boost::_bi::bind_t<const Buffer* const, boost::_mfi::cmf1<const Buffer* const, LyX, const InsetBase*>, boost::_bi::list2<boost::reference_wrapper<const LyX>, boost::_bi::value<const InsetBase*> > >]' ../../boost/boost/signals/slot.hpp:121: instantiated from 'boost::slot<SlotFunction>::slot(const F&) [with F = boost::_bi::bind_t<const Buffer* const, boost::_mfi::cmf1<const Buffer* const, LyX, const InsetBase*>, boost::_bi::list2<boost::reference_wrapper<const LyX>, boost::_bi::value<const InsetBase*> > >, SlotFunction = boost::function<void ()(), std::allocator<void> >]' render_graphic.C:44: instantiated from here ../../boost/boost/bind.hpp:1105: error: no matching function for call to 'visit_each(boost::signals::detail::bound_objects_visitor&, const InsetBase* const&, int)' This patch "fixes" the compile problems but we do not really want to apply it. Any opinions on this? Is it something strange hidden in our code, boost or a real regression in gcc? -- Lgb
"Lars Gullik Bjønnes" wrote:
Any opinions on this?
Is it something strange hidden in our code, boost or a real regression in gcc?
Can you try adding #include <boost/visit_each.hpp> somewhere at the top of boost/bind.hpp and see if this fixes things? You should probably test the CVS version as well.
"Peter Dimov" <pdimov@mmltd.net> writes: | "Lars Gullik Bjønnes" wrote: | | > Any opinions on this? | > | > Is it something strange hidden in our code, boost or a real | > regression in gcc? | | Can you try adding #include <boost/visit_each.hpp> somewhere at the top of | boost/bind.hpp and see if this fixes things? That seems to have fixed the problem. But I will run some further tests. | You should probably test the | CVS version as well. I have seen this problem with both 1.32 and 1.33[.1], but am a bit reluctant on using the CVS version (but I will test it) -- Lgb
larsbj@gullik.net (Lars Gullik Bjønnes) writes: | "Peter Dimov" <pdimov@mmltd.net> writes: | | | "Lars Gullik Bjønnes" wrote: | | | | > Any opinions on this? | | > | | > Is it something strange hidden in our code, boost or a real | | > regression in gcc? | | | | Can you try adding #include <boost/visit_each.hpp> somewhere at the top of | | boost/bind.hpp and see if this fixes things? | | That seems to have fixed the problem. | But I will run some further tests. Is this something that should be added on the 1.33 branch? (Is something like this in CVS already?) -- Lgb
"Lars Gullik Bjønnes" wrote:
larsbj@gullik.net (Lars Gullik Bjønnes) writes:
"Peter Dimov" <pdimov@mmltd.net> writes:
"Lars Gullik Bjønnes" wrote:
Any opinions on this?
Is it something strange hidden in our code, boost or a real regression in gcc?
Can you try adding #include <boost/visit_each.hpp> somewhere at the top of boost/bind.hpp and see if this fixes things?
That seems to have fixed the problem. But I will run some further tests.
Is this something that should be added on the 1.33 branch?
I don't know whether there would be a 1.33.2 now that we've already frozen for 1.34.
(Is something like this in CVS already?)
Not yet, I wanted you to confirm that it works. I'll add it now.
"Peter Dimov" <pdimov@mmltd.net> writes: | > Is this something that should be added on the 1.33 branch? | | I don't know whether there would be a 1.33.2 now that we've already frozen | for 1.34. I'd guess that there won't be a 1.33.2, but it would be nice if HEAD on the branch compiled with GCC 4.1. But no big deal. | > (Is something like this in CVS already?) | | Not yet, I wanted you to confirm that it works. I'll add it now. Super. -- Lgb
On Feb 20, 2006, at 12:22 PM, Lars Gullik Bjønnes wrote:
We se some strange problems with GCC 4.1 (the prerelease/release candidate) with use of boost::signal and boost::bind. Have anyone run the regressions tests for this GCC version?
g++ (GCC) 4.1.0 20060219 (prerelease
We get errors of this form: [snip] <gcc41-1.diff>
Any opinions on this?
Hmmm, I'm surprised that patch fixes the problem.
Is it something strange hidden in our code, boost or a real regression in gcc?
There is definitely a bug in Boost.CVS. However, I didn't think that GCC 4.1 should be susceptible to the bug; I remember fixing this bug for GCC 4.0.1, and it works there. So we might actually have two bugs; I really don't know The Boost bug is an interaction between the storage optimizations that went in to Bind (to make the storage optimizations of Function actually useful) and the visit_each mechanism. I've taken a stab at a fix (to Bind) that seems to solve the problem for me on GCC 3.3; it should solve the problems on other compilers as well. Attached is a patch to bind and a test case that illustrates the problem. Peter, could you take a look? Doug
Douglas Gregor wrote:
On Feb 20, 2006, at 12:22 PM, Lars Gullik Bjønnes wrote:
We se some strange problems with GCC 4.1 (the prerelease/release candidate) with use of boost::signal and boost::bind. Have anyone run the regressions tests for this GCC version?
g++ (GCC) 4.1.0 20060219 (prerelease
We get errors of this form: [snip] <gcc41-1.diff>
Any opinions on this?
Hmmm, I'm surprised that patch fixes the problem.
Is it something strange hidden in our code, boost or a real regression in gcc?
There is definitely a bug in Boost.CVS. However, I didn't think that GCC 4.1 should be susceptible to the bug; I remember fixing this bug for GCC 4.0.1, and it works there. So we might actually have two bugs; I really don't know
The Boost bug is an interaction between the storage optimizations that went in to Bind (to make the storage optimizations of Function actually useful) and the visit_each mechanism.
You're mixing these up. The bug in this thread is present in 1.33.1 which doesn't have the storage optimization. It only shows up on proper two-phase lookup compilers because the primary declaration of visit_each wasn't #included.
I've taken a stab at a fix (to Bind) that seems to solve the problem for me on GCC 3.3; it should solve the problems on other compilers as well.
Attached is a patch to bind and a test case that illustrates the problem. Peter, could you take a look?
The patch seems good to me. There are several things that I don't like in the test, though; (a) it doesn't actually test anything, (b) it uses boost/test instead of lightweight_test.hpp, (c) it binds std::plus<> with one argument, which is invalid and can be rejected by a conforming bind. I tried to fix it, but an interesting question has come up. The primary visit_each is defined as: template<typename Visitor, typename T> inline void visit_each(Visitor& visitor, const T& t); By forcing a const qualification on the visited object, this interface prevents a visitor from mutating it (in addition to breaking when a function is being passed for t.) Is this deliberate or accidental? I've also found bind_visitor.cpp. Looks like it expects a certain number of visitations, though, so the storage optimization will break it either way. Anyway. Could you please look at the attached test case and confirm that it passes with your patch applied?
On Feb 28, 2006, at 7:49 AM, Peter Dimov wrote:
Douglas Gregor wrote:
The Boost bug is an interaction between the storage optimizations that went in to Bind (to make the storage optimizations of Function actually useful) and the visit_each mechanism.
You're mixing these up. The bug in this thread is present in 1.33.1 which doesn't have the storage optimization. It only shows up on proper two-phase lookup compilers because the primary declaration of visit_each wasn't #included.
Ah, okay. In that case, I was fixing the other Signal/Bind bug that shows up in Boost CVS.
The patch seems good to me. There are several things that I don't like in the test, though; (a) it doesn't actually test anything, (b) it uses boost/test instead of lightweight_test.hpp, (c) it binds std::plus<> with one argument, which is invalid and can be rejected by a conforming bind.
You're right, of course. Thanks for the improved test case.
I tried to fix it, but an interesting question has come up. The primary visit_each is defined as:
template<typename Visitor, typename T> inline void visit_each(Visitor& visitor, const T& t);
By forcing a const qualification on the visited object, this interface prevents a visitor from mutating it (in addition to breaking when a function is being passed for t.) Is this deliberate or accidental?
I think it was deliberate when I originally implemented visit_each, because I wasn't imagining that it would be used for mutation. However, now I think it was the wrong decision... taking a non-const T& even has the side benefit of causing a failure if someone accidentally tries to stick a temporary into visit_each.
Anyway. Could you please look at the attached test case and confirm that it passes with your patch applied?
Yes, it passes on both GCC 4.0.1 and GCC 3.3. Doug
On Feb 28, 2006, at 4:37 PM, Peter Dimov wrote:
Douglas Gregor wrote:
Anyway. Could you please look at the attached test case and confirm that it passes with your patch applied?
Yes, it passes on both GCC 4.0.1 and GCC 3.3.
The test is in CVS now, please feel free to apply the patch, and thanks. :-)
Done. Doug
participants (3)
-
Douglas Gregor -
larsbj＠gullik.net -
Peter Dimov