Bug 31247 - std::vector::iterator::value_type is accessible
Summary: std::vector::iterator::value_type is accessible
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.3.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-03-17 16:52 UTC by Sylvain Pion
Modified: 2015-04-09 14:29 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
make nested iterator typedefs private in debug mode (720 bytes, patch)
2008-03-09 15:51 UTC, Jonathan Wakely
Details | Diff
new test (1001 bytes, text/x-c++src)
2008-03-09 15:52 UTC, Jonathan Wakely
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sylvain Pion 2007-03-17 16:52:36 UTC
This is an "accepts invalid code" report.

The following code happens to compile with g++ (all versions),
whereas the standard does not guarantee it should (I think).
----------------------------
#include <vector>

typedef std::vector<int>::iterator Iterator;

Iterator::value_type v;
Iterator::pointer p;
Iterator::iterator_category c;
----------------------------

The problem is that developing code with g++, and introducing this
kind of code by mistake, pops up errors later when other compilers
are used (e.g. those which have pointers for std::vector::iterator).

Would it be a good idea to make the access to such types somehow non public?
For example by making them protected and making std::iterator_traits a friend?
Or maybe only available when _GLIBCXX_DEBUG is not defined ?
Comment 1 Andrew Pinski 2007-03-17 17:00:07 UTC
IIRC iterator is an implemention defined type so this is valid.
Comment 2 Chris Jefferson 2007-03-17 17:19:32 UTC
Depending on how you read it, 24.3.1 looks to me like it might require that. It says that iterator_traits is defined to include:

typedef typename Iterator::value_type value_type;

and that it is specialised for various types of pointers. If we removed std::vector<int>::iterator::value_type, we would then have to overload iterator_traits for vector, and I'm not sure we are allowed to.

Out of interest, which compilers still have a pointer for std::vector::iterator?
Comment 3 Sylvain Pion 2007-03-17 17:35:16 UTC
I'm not sure about the standard requirements.  My "user question" is that
I want to develop with GCC as my main compiler, and I want it to catch
non-portability bugs as much as possible.

@Chris: note that std::iterator_traits would not need to be specialized for
std::vector::iterator if it was made a friend of it, so that it would be the
only one able to access these types.

The compiler I know which still has a pointer for std::vector::iterator is PGCC
(the Portland Group Compiler).
Comment 4 Chris Jefferson 2007-03-17 17:52:15 UTC
The main problem I can see with changing this is that you would have to decide if you were going to remove the same options from the iterators of all other standard containers. To not do so would seem to be inconsistent, but doing so would probably break a huge amount of code.

This seems to be part of a larger issue, should these typedefs be defined in any of the standard iterators. Having an "offical decision" by the standards committee would probably be useful.
Comment 5 Chris Jefferson 2007-04-17 16:48:07 UTC
I've done a bit of research into this. Looks like, as Andrew says the standard doesn't have much to say on the issue. Any method of removing this is going to make some other things a bit more messy I think, and I found a few pieces of code that use this and it seems unnesasary to break it.
Comment 6 Paolo Carlini 2007-04-17 17:04:41 UTC
I also had a look lately, and probably I'm coming to your same conclusions...
Comment 7 Jonathan Wakely 2008-03-09 15:51:59 UTC
Created attachment 15284 [details]
make nested iterator typedefs private in debug mode

It's "accepts implementation-defined" not "accepts invalid" - and it's obviously by design that GCC accepts its own implementation-defined constructs!

This patch makes the typedefs inaccessible in debug mode and fixes up a couple of functions that refer to them directly, rather than through iterator_traits.

Tested x86_64/linux. I also tried testing with -D_GLIBCXX_DEBUG added to testsuite_flags but got lots of spurious failures that I don't think are related to this change (as with parallel mode testing, all the 23_containers/*/synopsis.cc tests fail in debug mode - I haven't investigated why)
Comment 8 Jonathan Wakely 2008-03-09 15:52:58 UTC
Created attachment 15285 [details]
new test
Comment 9 Chris Jefferson 2008-03-09 20:28:32 UTC
Sorry to be pedantic, but could this be added to _GLIBCXX_DEBUG_PEDANTIC. I've previously tended to assume that _GLIBCXX_DEBUG should change only flag code that should fail in non-debug mode, but fails to be detected, whereas _GLIBCXX_DEBUG_PEDANTIC ensures that the code is standards compliant.

On another note, could this be added when just -pedantic is added? I think it's a useful flag to add, I'm just trying to avoid having to change a bunch of working, although non standards-complaint code :)
Comment 10 Marc Glisse 2011-10-28 09:31:30 UTC
Just noticed this accidentally while looking for something else. And I am opposed to hiding the standard typedefs (particularly iterator_category), even in some debug mode. An iterator is either a pointer or a class with the typedefs. If you want to portably detect iterators (for sfinae purposes), that's exactly what you'll test, since iterator_traits is not guaranteed to be sfinae-friendly.

On the other hand, I would not be opposed to a signature: iterator& operator--()&; for the decrement operator (notice the final '&') when support appears in the compiler. Next time I read --v.end()...

Note that IIRC Howard's libc++ uses pointers, which should make those issues more visible.
Comment 11 Paolo Carlini 2011-10-28 09:40:36 UTC
Interesting, thanks.

By the way, I would guess Sylvain' email doesn't work anymore, thus it's unlikely that he can give us his feedback ;) (or does he have a forward in place, as far as you know?)
Comment 12 Jonathan Wakely 2011-10-28 09:57:01 UTC
(In reply to comment #10)
> An iterator is either a pointer or a class with the
> typedefs.

Or a type for which iterator_traits has been specialized?

I'm not really interested in the patch, I did it as a proof of concept and reverted it in my tree a long time ago
Comment 13 Marc Glisse 2011-10-28 10:18:43 UTC
(In reply to comment #12)
> (In reply to comment #10)
> > An iterator is either a pointer or a class with the
> > typedefs.
> 
> Or a type for which iterator_traits has been specialized?

Yes. Sadly, that's not portably detectable. The 2 categories above are those that can be tested. And having a private iterator_category completely breaks sfinae in C++03.
Comment 14 Sylvain Pion 2011-10-29 04:22:49 UTC
(@Paolo : I still receive email at my old address, so far)

I don't have a strong opinion on this.  It's certainly nice to have a strongly compliant mode to make it easier to write portable code, although that doesn't replace proper testing with other compilers anyway.  I understand that there can be a small drawback here, and maybe not a lot of motivation to dig further on this particular question.
Comment 15 Jonathan Wakely 2014-09-19 10:33:08 UTC
(In reply to Marc Glisse from comment #10)
> that's exactly what you'll test, since iterator_traits is not guaranteed to
> be sfinae-friendly.

N.B. that's changed for C++14.

I'm still not motivated to implement this request though.
Comment 16 Marc Glisse 2014-09-19 11:37:12 UTC
(In reply to Jonathan Wakely from comment #15)
> I'm still not motivated to implement this request though.

It would break too much code.

If people really insisted (which they don't), I believe it would be safer to specialize iterator_traits and _remove_ the nested vector::iterator::* typedefs than to make them private, since access control in C++ is a trap.
Comment 17 Jonathan Wakely 2015-04-09 14:29:03 UTC
Noone is convinced this would be a positive change, let's close it.