This is the mail archive of the libstdc++@sourceware.cygnus.com mailing list for the libstdc++ project. See the libstdc++ home page for more information.
Oleg wrote: > Nathan Myers <ncm@cygnus.com> wrote: > : Oleg Zabluda wrote: > : > Ryszard Kabatek <rysio@rumcajs.chemie.uni-halle.de> wrote: > : > : I wrote an iterator class for std::vector<> and std::basic_string<>. > ... > : > : explicit _Iterator(pointer __p); > : > > : > I think _Iterator::_Iterator(pointer __p) should be private. Other > : > then plain aesthetics, this will also prevent users from writing > : > non-compliant code like > : > > : > vector<int>::iterator i (0); > : > : Agree. Probably it should take a dummy second argument of a > : type in the restricted namespace. > > Why? It will be ``explicit'' and private. What am I missing? Private constructors still participate in overloading. > : The only class that should be > : able to call the ctor should be vector. > > And possibly the const cousin. Right. > : > : template<typename _Up> > : > : _Iterator(const _Iterator<_Tp, _Dist, _Up*, _Up&>& __rhs); > : > > : > That's very unsafe. This will provide a conversion not only > : > from iterator to const_iterator but many other undesirable > : > _implicit_ conversions as well. For example, > : > vector<Derived>::iterator --> vector<Base>::iterator. > > : No, this is not a conversion from a const_iterator, it's just > : a copy of a const reference to an ordinary iterator. > > I think you are mistaken. Of course you are right. > BTW. Now that I look at this again, I see other problems with > this piece of code. First of all, technicaly, this is not just > a copy constructor, therefore the comment is incorrect. Second of all, > there is no matching [non-copy] assignment operator. In fact it's not a copy constructor at all; given this definition the compiler would still automatically generate a copy constructor. I agree that we don't want any promiscuously polymorphic copy constructors for vector and string, because the types they store cannot be used polymorphically. The assignment would also be compiler-generated. > Third of all, -Weffc++ will complain that we have a pointer > data member, but do not define our own copy constructor and > copy assignment. I am afraid, currently we have no choice but to > define them, if we want to have any hope to ever have our headers be > -Weffc++ clean. Amen. I am afraid though that this might lead > to a performance penalty of some sort. Comments anyone? I don't give a rip about -Weffc++ cleanliness. Those rules are not generally-applicable enough to apply it uniformly to the standard library. I don't mind running things past it to see what happens, but I refuse to be bound by it. Scott Meyers conflates "OO design" with "C++ coding", but they are different processes with different rules. When you're not coding an OO design, OO rules don't necessarily apply. > The ideal solution would be to surgically surpress those warnings > with some king of #pragma, but the last time Mark Mitchell was > proposing introducing them, other disagreed that it would be worth it. I agree. They are just guidelines. > : > Making iterator and const_iterator separate classes (see above) > : > will solve this. > > : Yes, we do need separate classes for iterator and const_iterator. > : Derivation makes this easy: the base class should be named > : const_iterator, and the derived just iterator. Slicing is your > : friend! :-) > > Actually, it is always your enemy :-) I can't think of a single > example where it would be your friend. > > We should not derive iterator from const_iterator. > In this particular case, it is impossible to do that and stay > -Weffc++ clean, because, for example, you will not be able > to override operator*(). At best you will hide it. What kind > of derivation is that? Moreover, making the two really, really > different classes, we can make the pointer data member in the > const_iterator to be a real pointer to const. Besides, if > we derive one from the other, and not explicitly ban the slicing, > we will introduce other bizarre _implicit_ conversions, like > iterator* --> const_iterator* (compare to T** --> const T**). > > In short, we'll never drive Plauger out of business if we > gonna repeat his mistakes :-) OK, agreed. I hadn't studied his mistakes beyond his having derived in the wrong direction (!). I would like to do some experiments with private derivation and some "using" exposures. The nice properties of built-in promotion are not to be given up lightly. > We could derived them from a common base, but this will > introduce a vtbl and we don't want that. Right, maybe. Anyway, for something as simple as a pointer wrapper I'm not sure implementation-inheritance would buy you anything anyway, besides built-in promotion. > : > Also -- general understanding is that Iterators are typically > : > designed so that they can be very efficiently passed by value > : > (ours are). And they are absolutely not supposed to be used > : > polymorphically. I think we should lose passing by reference > : > altogether. > > : Except of course in the copy constructor. :-) > : We don't actually need to define one; the language provides > : just the right semantics by default. > > Right. We shouldn't. Unfortunately we might be forced to. See above. A converting constructor, OK. But that's different. > : > This is somewhat controvercial, but I think we should prepend `this->'' > : > in front of all member variables and functions, like this: > : > > : > return *(this->_M_data); > : > > : > This will reduce the risk great deal. Imagine somebody doing > : > #define _M_data 0 > > : That #define is strictly forbidden; if users do it, they should expect > : breakage. > > Right. But if we can help it, with absolutely no downside, why > not? Especially since, strictly speaking, we invade namespace > of other compilers. If they define _M_data, all kinds of things break. That's the last thing we can afford to worry about. But still, it would be no bad thing to use this-> for member access. > : However, all references to member names not preceded by > : "_M" or "_S" should be qualified this way. I.e. we try to use > : "this->size()" in basic_string<> members in the library. The > : convention is not followed with any regularity, but should be. > : Patches which fix such lapses are welcome. > > : The reason is not for protection against macros, but to protect against > : surprises in two-phase lookup. > > Right. I just thought that macro was a simpler example. The > case in question seemed specially heinous, because that would > lead to returning a null pointer from a valid iterator, core > dumps, and absolutely no sane way to debug the problem that > I can think of. That would be the least of the problems if somebody did that. Anyway, any place where we had "_M_data = __p" would break too, and visibly. > : For those of you who have not followed > : comp.std.c++, here is a detailed explanation: > > : http://x13.dejanews.com/getdoc.xp?AN=391110976 > > : In the library we have not used "this->" syntax to refer to > : "_M_" prefix names, because those will never be global names > : and thus cannot be confused with the member names by two-stage > : lookup. > > Unless the user will. Or some other libraries will. Like I said, > it doesn't cost us anything, so why not? Users and other libraries are not allowed to define _M_ names. Of course vendor C headers might, but if we did then "this->" wouldn't help us anyway. Take a look in BADNAMES. More will be added to this, I am sure. > : When two-phase lookup is implemented lots of code will break. > : I think there will be a warning in the next Egcs snapshot for this. > > I am pretty scared of the name lookup issues myself. ``this->'' > makes me feel somewhat better. But I don't assume this is > universal, and would not insist, unless enough of other people > feel the same way. Personally, I think we should accept patches which add "this->" to template member name usage, but I will find it hard to add it universally. That may change. Nathan Myers ncm@cantrip.org