You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Travis Vitek <Tr...@roguewave.com> on 2008/03/10 19:35:53 UTC
RE: svn commit: r635439 - /stdcxx/trunk/include/string.cc
>Author: faridz
>Date: Sun Mar 9 22:52:37 2008
>New Revision: 635439
>
>URL: http://svn.apache.org/viewvc?rev=635439&view=rev
>Log:
>2008-03-10 Farid Zaripov <fa...@epam.com>
>
> * include/string.cc (__replace_aux): Use static_cast<>()
> instead of reinterpret_cast<>() to prevent compile error
> when _InputIter::operator*() returns rvalue.
>
>Modified:
> stdcxx/trunk/include/string.cc
>
>Modified: stdcxx/trunk/include/string.cc
>URL:
>http://svn.apache.org/viewvc/stdcxx/trunk/include/string.cc?rev
=635439&r1=635438&r2=635439&view=diff
>===============================================================
>===============
>--- stdcxx/trunk/include/string.cc (original)
>+++ stdcxx/trunk/include/string.cc Sun Mar 9 22:52:37 2008
>@@ -652,7 +652,7 @@
>
> if (__n2) {
> const const_pointer __ptr =
>- &_RWSTD_REINTERPRET_CAST
>(const_reference, *__first2);
>+ &_RWSTD_STATIC_CAST (const_reference, *__first2);
>
> if (__s.data () <= __ptr && __s.data () +
>__ssize > __ptr) {
> const _RWSTD_SIZE_T __tmp_size = __n2 *
>sizeof (value_type);
>
>
>
Farid,
Is this actually safe, and does it actually do what you want? If
_InputIter::operator* returns a rvalue and you static_cast that to a
const_reference, you will get the address of the temporary. That address
can't reliably be used to to test that `__first2' is inside `__s' or
not, so the bug that you are trying to eliminate here will still exist
for iterators of this type. Here is a complete testcase.
template <class _TypeT>
struct dumb_forward_iterator
{
dumb_forward_iterator (_TypeT* p) : ptr (p) { }
// generated...
//dumb_forward_iterator (const dumb_forward_iterator& other);
//
// generated...
//dumb_forward_iterator&
//operator= (const dumb_forward_iterator& other);
_TypeT operator* () { return *ptr; }
_TypeT operator* () const { return *ptr; }
_TypeT* operator-> () { return ptr; }
const _TypeT* operator-> () const { return ptr; }
dumb_forward_iterator& operator++ ()
{ ++ptr; return *this; }
dumb_forward_iterator operator++ (int)
{
dumb_forward_iterator tmp (*this);
++ptr;
return tmp;
}
_TypeT* ptr;
};
template <class _TypeT>
bool operator== (const dumb_forward_iterator<_TypeT>& lhs,
const dumb_forward_iterator<_TypeT>& rhs)
{
return lhs.ptr == rhs.ptr;
}
template <class _TypeT>
bool operator!= (const dumb_forward_iterator<_TypeT>& lhs,
const dumb_forward_iterator<_TypeT>& rhs)
{
return lhs.ptr != rhs.ptr;
}
#include <stdlib.h>
#include <stdio.h>
template <class _CharT>
struct string
{
typedef _CharT value_type;
typedef _CharT* iterator;
typedef const _CharT* const_iterator;
typedef _CharT& reference;
typedef const _CharT& const_reference;
typedef _CharT* pointer;
typedef const _CharT* const_pointer;
string (const _CharT* s)
{
int n = 0;
while (s [n])
++n;
buf = (_CharT*)malloc (n + 1);
size = n;
for (int m = 0; m < n; ++m)
buf [m] = s [m];
}
~string ()
{
free (buf);
}
iterator begin ()
{
return buf;
}
iterator end ()
{
return buf + size;
}
_CharT* buf;
int size;
};
template <class _CharT, class _InputIter>
void
__replace_aux (string<_CharT>& __s, typename
string<_CharT>::iterator __first1,
typename string<_CharT>::iterator __last1,
_InputIter __first2, _InputIter __last2)
{
const string<_CharT>::const_pointer __ptr =
&static_cast<string<_CharT>::const_reference>(*__first2);
if (__s.buf <= __ptr && __ptr < __s.buf + __s.size)
printf ("__first2 is inside __s\n");
else
printf ("__first2 is *not* inside __s\n");
}
int main ()
{
string<char> s ("abc");
dumb_forward_iterator<string<char>::value_type> dbeg (s.begin
());
dumb_forward_iterator<string<char>::value_type> dend (s.end
());
__replace_aux (s, s.begin (), s.end (), dbeg, dend);
return 0;
}
If _InputIter is a pointer type, you should not need a cast at all, and
if it is a class type, you _should_ be able to use the result of
operator->(), but that isn't really safe because it could return a proxy
for the actual pointer. I think you could use SFINAE and traits to
detect that the return type of operator->() is a pointer type if we
really wanted to, and then just make a copy if it isn't.
Travis
RE: svn commit: r635439 - /stdcxx/trunk/include/string.cc
Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Travis Vitek [mailto:Travis.Vitek@roguewave.com]
> Sent: Monday, March 10, 2008 8:36 PM
> To: dev@stdcxx.apache.org
> Subject: RE: svn commit: r635439 - /stdcxx/trunk/include/string.cc
>
> Is this actually safe, and does it actually do what you want? If
> _InputIter::operator* returns a rvalue and you static_cast
> that to a const_reference, you will get the address of the
> temporary. That address can't reliably be used to to test
> that `__first2' is inside `__s' or not, so the bug that you
> are trying to eliminate here will still exist for iterators of this
type.
Yes, it was what I want. Actually I don't think we need detect
the self reference for all posible iterators, that could be passed in
string methods. For example the STLport checking the only if
_InputIterator type is string::iterator (which is equal to char_type*).
I think that checking the iterator and reverse_iterator<iterator> types
is enough. What do you think?
> If _InputIter is a pointer type, you should not need a cast
> at all, and if it is a class type, you _should_ be able to
> use the result of
> operator->(), but that isn't really safe because it could
> return a proxy
> for the actual pointer. I think you could use SFINAE and
> traits to detect that the return type of operator->() is a
> pointer type if we really wanted to, and then just make a
> copy if it isn't.
Yes, I agree that using the type traits is more suitable solutuion.
I'm not used the type traits in this patch because as Martin said
in
http://www.mail-archive.com/stdcxx-dev@incubator.apache.org/msg05705.htm
l
that the changes which are using the type traits should be holded until
4.3 release. Another option is adding the overloads for assign(),
replace(), insert()
methods, but this is not a binary compatible change.
Farid.