After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 543671 - Notebook::PageList::const_iterator operator!= does not work (with gcc 4.3.1)
Notebook::PageList::const_iterator operator!= does not work (with gcc 4.3.1)
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: general
2.12.x
Other All
: Normal minor
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2008-07-18 23:29 UTC by Julien Langer
Modified: 2010-10-19 17:10 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
notebook_pages_const_iter_test.cc (408 bytes, text/plain)
2008-08-04 12:04 UTC, Murray Cumming
Details
Source code for Gtk::Notebook test case (51.23 KB, text/plain)
2010-10-19 12:10 UTC, Kjell Ahlstedt
Details

Description Julien Langer 2008-07-18 23:29:42 UTC
The following code does not work with gtkmm 2.12.7 and gcc 4.3.1

const Notebook::PageList& list = notebook->pages();
Notebook::PageList::const_iterator it = list.begin();
for ( ; it != list.end(); ++it ) { ... }

Compilation fails on containers.h:340
bool operator!=(const Self& src) const { return T_Base::operator!=(src); }

with the following error from gcc:
/usr/include/glibmm-2.4/glibmm/containers.h|340|error: 'operator!=' is not a member of 'Gtk::Notebook_Helpers::PageIterator'|

It works fine when the const_iterator is replaced with a plain iterator.
Comment 1 Julien Langer 2008-07-18 23:40:49 UTC
Well, const_iterator doesn't seem to be the only iterator type that's not working:
reverse_iterator and const_reverse_iterato produce the same error.
Comment 2 Murray Cumming 2008-08-04 12:03:14 UTC
This seems to be because the PageList has the operator== and operator != as
separate functions rather than member functions:

/** @relates Gtk::Notebook_Helpers::PageIterator */
inline bool operator==(const PageIterator& lhs, const PageIterator& rhs)
  { return lhs.equal(rhs); }

/** @relates Gtk::Notebook_Helpers::PageIterator */
inline bool operator!=(const PageIterator& lhs, const PageIterator& rhs)
  { return !lhs.equal(rhs); }


I forget why that's generally considered a good thing.

I suppose we could make the member operator functions separate functions in
glibmm/containers.h and make the code expect that.

Thoughts?
Comment 3 Murray Cumming 2008-08-04 12:04:08 UTC
Created attachment 115815 [details]
notebook_pages_const_iter_test.cc

An actual test case.
Comment 4 Julien Langer 2008-08-04 16:52:45 UTC
non-member operators are usually considered a good thing because it's the only way to support implicit type conversions for both operands.

It helps to replace containers.h:339 and 340:
  bool operator==(const Self& src) const { return T_Base::operator==(src); }
  bool operator!=(const Self& src) const { return T_Base::operator!=(src); }

with the following:
  bool operator==(const Self& src) const { return *this == src }
  bool operator!=(const Self& src) const { return *this != src }
Comment 5 Murray Cumming 2008-08-05 13:08:49 UTC
Wouldn't that be an infinite loop?

Could you try that and provide a patch, please?
Comment 6 Julien Langer 2008-08-05 14:21:45 UTC
Yes, you're right it's an infinite loop.

Why are those operators defined there anyhow?
If I just remove the operators from class List_ConstIterator then the correct non-member operator is invoked.

Does this extra indirection have any benefit?
Comment 7 Murray Cumming 2008-08-05 14:27:41 UTC
I guess they are there because there would be no == or != without them, because there is no separate static == or !- operator overloading for those types (though there are for the notebook stuff).

I guess it wouldn't be too bad to change them to non-member operator overloads in containers.h. I guess that not many people are overriding them, call these member methods explicitly.
Comment 8 Julien Langer 2008-08-05 14:48:16 UTC
Do you mean to define a non-member operator for List_ConstIterator?
How would that help? The compiler would then still select this operator over the one defined for const PageIterator&
Comment 9 Murray Cumming 2008-08-06 06:38:20 UTC
Yeah.

Maybe we can specify the exact operator= by casting to the specific type? But maybe that would still sometimes result in an infinite loop.
Comment 10 Julien Langer 2008-08-06 08:05:36 UTC
Hm.. casting to the concrete type wouldn't work in all cases, for example when the operator is defined in another base class (ok this also wouldn't work with the current solution)

Wouldn't be the best solution to make all operators in all the classes non-member operators and remove the operators from containers.h?
That way the compiler could always select the best match via overload resolution and implicit conversions. It would also not break existing code.
And subclasses could still provide their own version of the operator by just providing an operator with the concrete type of that subclass.
Or am I missing something?
Comment 11 Murray Cumming 2008-08-06 08:26:35 UTC
> Wouldn't be the best solution to make all operators in all the classes 
> non-member operators and remove the operators from containers.h?

Remove the operators? That would definitely break API.

Of course, I'd be very happy if you tried creating a patch to test your idea.
Comment 12 Kjell Ahlstedt 2010-10-19 12:10:07 UTC
Created attachment 172705 [details]
Source code for Gtk::Notebook test case

In comments 4, 5, and 6 we learn that it's not possible to replace:
  bool operator==(const Self& src) const { return T_Base::operator==(src); }
  bool operator!=(const Self& src) const { return T_Base::operator!=(src); }

with the following:
  bool operator==(const Self& src) const { return *this == src }
  bool operator!=(const Self& src) const { return *this != src }

But it's possible to replace with:
  bool operator==(const Self& src) const
  { return *static_cast<const T_Base*>(this) == src; }
  bool operator!=(const Self& src) const
  { return *static_cast<const T_Base*>(this) != src; }

Then operator== and operator!= can be either member functions in PageIterator,
or they can be non-member functions. See the attached test case, with
comments at the beginning of the attachment.

I've seen in the Git repository that Gtk::Notebook has been considerably
modified since gtkmm v2.20.3. That's what's included in Ubuntu 10.10, which
I'm using. In branch gtkmm-2-22 PageIterator is marked deprecated, and in the
master branch it's removed. So perhaps this comment is too late to be useful.
Is the solution of the problem with the iterators to simply remove them?
Comment 13 Murray Cumming 2010-10-19 17:10:18 UTC
(In reply to comment #12)
> Is the solution of the problem with the iterators to simply remove them?

Sorry yes, they have been removed in gtkmm 3 (2.91.x so far). They can only be brought back if someone adds support for it in the GTK+ C API. They were using direct struct access which is now (wisely) impossible.