Boost C++ Libraries: Ticket #5697: iterator_facade::operator-> is broken for proxy references

Dave Abrahams — Thu, 14 Jul 2011 20:26:11 GMT

Please do submit a patch; that will make the report much easier to evaluate.

Thanks!

attachment set

Fri, 15 Jul 2011 01:15:07 GMT

attachment → iterator_facade.patch.hpp

Fri, 15 Jul 2011 01:18:12 GMT

I'm a novice at creating patches; hopefully iterator_facade.patch.hpp is sufficient, and if not, let me know. I changed iterator_facade as indicated in the patch and, at least, everything built correctly. If you want, I can try figuring out how to run the Boost.Iterator tests and verify that they all pass (or not).

Fri, 15 Jul 2011 01:19:02 GMT

"built correctly" == "without compiler errors" :/

Dave Abrahams — Fri, 15 Jul 2011 01:33:21 GMT

please run bjam in libs/iterator/test under BOOST_ROOT and see if any tests fail.

attachment set

Fri, 15 Jul 2011 03:12:52 GMT

attachment → iterator_facade.patch.2.hpp

Fri, 15 Jul 2011 03:21:01 GMT

Okay, I ran the iterator tests, and iterator_facade.cpp failed to compile at line 107, "p->mutator();". To get that line to compile (and the test to pass), I removed some consts from the operator_arrow_dispatch::proxy, as shown in the second attachment (iterator_facade.patch.2.hpp). Looks like all iterator tests pass now, but I'm not really sure if the consts would be necessary on other compilers (I'm using MSVC9 on Windows 7). Let me know what you think when you get a chance (and thanks for looking at this with me!).

Dave Abrahams — Fri, 15 Jul 2011 23:44:31 GMT

(minor detail: next time make the attachments end with .patch instead of .hpp and they will render as patches.)

Dave Abrahams — Fri, 15 Jul 2011 23:54:21 GMT

Your patch looks excellent. I don't think there's any reason to use implicit_cast in there, though. At this point, it should just be

return boost::addressof(x);

If you can fix that, and add a test case that fails without your patch and passes otherwise, I'll be happy to commit this.

anonymous — Sat, 16 Jul 2011 00:16:36 GMT

Agreed on both accounts; I was just trying to maintain the same style as before.

I will change the patch accordingly (with a .patch extension) and add a test case to...iterator_facade.cpp, I suppose?

attachment set

Sun, 17 Jul 2011 03:37:41 GMT

attachment → iterator_facade.hpp.patch

Sun, 17 Jul 2011 03:42:26 GMT

I have uploaded a patch for iterator_facade.hpp, but I'm having issues uploading a patch for iterator_facade.cpp (from libs/iterator/test). The spam filter keeps thinking I'm uploading spam :( I guess I will email the attachment directly to you, Dave, and if it gets lost in the shuffle, I can try uploading again...

The test fails for the present version of iterator_facade with

compile-c-c++ ..\..\..\bin.v2\libs\iterator\test\iterator_facade.test\msvc-9.0\debug\threading-multi\iterator_facade.obj iterator_facade.cpp D:\boost_1_47_0\boost/iterator/iterator_facade.hpp(327) : error C2664: 'boost::implicit_cast' : cannot convert parameter 1 from 'wrapper<T> *' to 'boost::detail::operator_arrow_proxy<T>'

with [

T=int &

] and [

T=wrapper<int>

] No constructor could take the source type, or constructor overload resolution was ambiguous

...as expected.

attachment set

Mon, 18 Jul 2011 03:37:22 GMT

attachment → iterator_facade.cpp.patch

Sun, 07 Aug 2011 02:41:19 GMT

:: ping ::

How likely will this sneak into 1.48?

anonymous — Wed, 02 Nov 2011 13:23:23 GMT

Jeff, is this earlier thread describing the same problem?

http://thread.gmane.org/gmane.comp.lib.boost.devel/198562

Wed, 02 Nov 2011 16:40:25 GMT

Replying to anonymous:

Jeff, is this earlier thread describing the same problem?

http://thread.gmane.org/gmane.comp.lib.boost.devel/198562

I believe so.

Jeremiah Willcock — Sat, 31 Mar 2012 23:39:43 GMT

I think that bug #4313 is reporting the same issue as well, and also has a patch. Should I close that one as a duplicate now?

Mon, 02 Apr 2012 17:45:21 GMT

Looks like it, so yes, you should probably close #4313 as duplicate. Feel free to apply the attached patches, although I'm hoping to have some free time in the coming months to actually learn the development process and get this and other Iterator tickets taken care of.

Jeremiah Willcock — Mon, 02 Apr 2012 18:02:35 GMT

Which of the patches would you like me to apply? One of them from this ticket or the one from #4313? I'll go ahead and close #4313 anyway.

Mon, 02 Apr 2012 18:14:30 GMT

If willing, please apply iterator_facade.*pp.patch from this ticket (the latter 2 patches). I believe they are more complete than the patch provided in #4313.

Jeremiah Willcock — Mon, 02 Apr 2012 18:19:46 GMT

(In [77723]) Applied patches from #5697; refs #5697

Jeremiah Willcock — Mon, 02 Apr 2012 18:21:22 GMT

I've applied the patches you wanted -- please close the bug if you think it works now.

Mon, 02 Apr 2012 18:24:15 GMT

Awesome, thanks. Once I reset my SVN password (::sigh::) I'll review the test result matrix and close this ticket.

status changed; resolution set

jeffrey.hellrung — Tue, 17 Apr 2012 14:35:03 GMT

status new → closed
resolution → fixed

jeffrey.hellrung — Tue, 24 Apr 2012 21:28:14 GMT

(In [78184]) merging from trunk; fix #5127 from M. Morin; fix for refs #5697