You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2007/10/18 00:12:31 UTC
[PATCH] std::string::_C_grow() 4.2 incompatibility (was: Re: difference
in exported symbols between 4.1.3 and 4.2.0 15d dll's (MSVC 7.1))
Martin Sebor wrote:
> Some more wisdom that I had snipped previously:
>
> Travis Vitek wrote:
> [...]
> >
> > Note that the basic_string<>::_C_grow() method is not accessed by any
> > inline member functions, but it still causes problems. I don't think we
> > can skip fixing these problems just because a function appears to not be
> > accessable to user code.
> >
>
> Ah, I know why! Because it's called from a member template which
> gets instantiated in the test.
>
> So we need to add string::_C_grow() to the list of symbols to
> fix in 4.2.0.
Travis, can you try this patch?
Index: include/string
===================================================================
--- include/string (revision 585584)
+++ include/string (working copy)
@@ -504,8 +504,6 @@
return _C_make_iter (_C_data + __pos1);
}
-public:
-
#ifndef _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
template <class _InputIter>
@@ -519,6 +517,8 @@
return __rw_replace_aux (*this, __first1, __last1, __first2,
__last2);
}
+public:
+
#endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
size_type _C_grow (size_type, size_type) const;
Re: [PATCH] std::string::_C_grow() 4.2 incompatibility
Posted by Martin Sebor <se...@roguewave.com>.
Martin Sebor wrote:
> Travis Vitek wrote:
>> Martin,
>>
>> That patch won't work. The _C_grow needs to be made private. It was
>> private in 4.1.3, but is public in 4.2. IMO it should always be private
>> and it was an oversight that under some conditions it could be made
>> public.
>>
>> I have tested the following patch.
>
> I think they're the same as far as MSVC goes. Mine moves the
> public: above the conditional declaration of the __replace_aux()
> member template.
Crud! If the patch actually did what I said it would have been
good. Instead, I moved the public: *below* the conditional
declaration of __replace_aux(), thus making the member function
template private and inaccessible from the non-member function
__rw_replace_aux(), exactly what I was trying to avoid doing
in order to prevent breaking HP aCC. And sure enough, now that
I've tested it (but of course, not before checking it in), the
change breaks the compiler. Arrrgh!
Corrected as follows:
http://svn.apache.org/viewvc?rev=586162&view=rev
Martin
> The block that includes the public declaration
> is disabled for MSVC 7 and up, so the private: specifier right
> above it is in effect. No?
>
> The problem with your patch as I see it is that the
> __rw_replace_aux() (non-member) function template that's called
> from the __replace_aux() inline member template won't be able to
> access the (unconditionally) private _C_grow(). I think HP aCC
> on PA-RISC actually uses this block to work around a compiler
> bug (see http://issues.apache.org/jira/browse/STDCXX-271).
>
> Martin
>
>>
>>
>> Index: string
>> ===================================================================
>> --- string (revision 585687)
>> +++ string (working copy)
>> @@ -521,6 +521,8 @@
>>
>> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>
>> +private:
>> +
>> size_type _C_grow (size_type, size_type) const;
>>
>> public:
>>
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Martin Sebor [mailto:sebor@roguewave.com] Sent: Wednesday,
>>> October 17, 2007 3:13 PM
>>> To: stdcxx-dev@incubator.apache.org
>>> Subject: [PATCH] std::string::_C_grow() 4.2 incompatibility (was: Re:
>>> difference in exported symbols between 4.1.3 and 4.2.0 15d dll's
>>> (MSVC 7.1))
>>>
>>> Martin Sebor wrote:
>>>> Some more wisdom that I had snipped previously:
>>>>
>>>> Travis Vitek wrote:
>>>> [...]
>>>> >
>>>> > Note that the basic_string<>::_C_grow() method is not
>>> accessed by any
>>>> > inline member functions, but it still causes problems. I
>>> don't think we
>>>> > can skip fixing these problems just because a function
>>> appears to not be
>>>> > accessable to user code.
>>>> >
>>>>
>>>> Ah, I know why! Because it's called from a member template which
>>>> gets instantiated in the test.
>>>>
>>>> So we need to add string::_C_grow() to the list of symbols to
>>>> fix in 4.2.0.
>>> Travis, can you try this patch?
>>>
>>> Index: include/string
>>> ===================================================================
>>> --- include/string (revision 585584)
>>> +++ include/string (working copy)
>>> @@ -504,8 +504,6 @@
>>> return _C_make_iter (_C_data + __pos1);
>>> }
>>>
>>> -public:
>>> -
>>> #ifndef _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>>
>>> template <class _InputIter>
>>> @@ -519,6 +517,8 @@
>>> return __rw_replace_aux (*this, __first1, __last1, __first2,
>>> __last2);
>>> }
>>>
>>> +public:
>>> +
>>> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>>
>>> size_type _C_grow (size_type, size_type) const;
>>>
>>>
>
RE: [PATCH] std::string::_C_grow() 4.2 incompatibility
Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com]
> Sent: Thursday, October 18, 2007 5:38 PM
> To: stdcxx-dev@incubator.apache.org
> Subject: Re: [PATCH] std::string::_C_grow() 4.2 incompatibility
>
> I'm getting ready to commit the original patch -- Travis, do
> you agree? Farid, any concerns or comments with it?
The patch is ok.
Farid.
Re: [PATCH] std::string::_C_grow() 4.2 incompatibility
Posted by Martin Sebor <se...@roguewave.com>.
I'm getting ready to commit the original patch -- Travis,
do you agree? Farid, any concerns or comments with it?
Martin
Martin Sebor wrote:
> Travis Vitek wrote:
>> Martin,
>>
>> That patch won't work. The _C_grow needs to be made private. It was
>> private in 4.1.3, but is public in 4.2. IMO it should always be private
>> and it was an oversight that under some conditions it could be made
>> public.
>>
>> I have tested the following patch.
>
> I think they're the same as far as MSVC goes. Mine moves the
> public: above the conditional declaration of the __replace_aux()
> member template. The block that includes the public declaration
> is disabled for MSVC 7 and up, so the private: specifier right
> above it is in effect. No?
>
> The problem with your patch as I see it is that the
> __rw_replace_aux() (non-member) function template that's called
> from the __replace_aux() inline member template won't be able to
> access the (unconditionally) private _C_grow(). I think HP aCC
> on PA-RISC actually uses this block to work around a compiler
> bug (see http://issues.apache.org/jira/browse/STDCXX-271).
>
> Martin
>
>>
>>
>> Index: string
>> ===================================================================
>> --- string (revision 585687)
>> +++ string (working copy)
>> @@ -521,6 +521,8 @@
>>
>> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>
>> +private:
>> +
>> size_type _C_grow (size_type, size_type) const;
>>
>> public:
>>
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: Martin Sebor [mailto:sebor@roguewave.com] Sent: Wednesday,
>>> October 17, 2007 3:13 PM
>>> To: stdcxx-dev@incubator.apache.org
>>> Subject: [PATCH] std::string::_C_grow() 4.2 incompatibility (was: Re:
>>> difference in exported symbols between 4.1.3 and 4.2.0 15d dll's
>>> (MSVC 7.1))
>>>
>>> Martin Sebor wrote:
>>>> Some more wisdom that I had snipped previously:
>>>>
>>>> Travis Vitek wrote:
>>>> [...]
>>>> >
>>>> > Note that the basic_string<>::_C_grow() method is not
>>> accessed by any
>>>> > inline member functions, but it still causes problems. I
>>> don't think we
>>>> > can skip fixing these problems just because a function
>>> appears to not be
>>>> > accessable to user code.
>>>> >
>>>>
>>>> Ah, I know why! Because it's called from a member template which
>>>> gets instantiated in the test.
>>>>
>>>> So we need to add string::_C_grow() to the list of symbols to
>>>> fix in 4.2.0.
>>> Travis, can you try this patch?
>>>
>>> Index: include/string
>>> ===================================================================
>>> --- include/string (revision 585584)
>>> +++ include/string (working copy)
>>> @@ -504,8 +504,6 @@
>>> return _C_make_iter (_C_data + __pos1);
>>> }
>>>
>>> -public:
>>> -
>>> #ifndef _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>>
>>> template <class _InputIter>
>>> @@ -519,6 +517,8 @@
>>> return __rw_replace_aux (*this, __first1, __last1, __first2,
>>> __last2);
>>> }
>>>
>>> +public:
>>> +
>>> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>>
>>> size_type _C_grow (size_type, size_type) const;
>>>
>>>
>
Re: [PATCH] std::string::_C_grow() 4.2 incompatibility
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> Martin,
>
> That patch won't work. The _C_grow needs to be made private. It was
> private in 4.1.3, but is public in 4.2. IMO it should always be private
> and it was an oversight that under some conditions it could be made
> public.
>
> I have tested the following patch.
I think they're the same as far as MSVC goes. Mine moves the
public: above the conditional declaration of the __replace_aux()
member template. The block that includes the public declaration
is disabled for MSVC 7 and up, so the private: specifier right
above it is in effect. No?
The problem with your patch as I see it is that the
__rw_replace_aux() (non-member) function template that's called
from the __replace_aux() inline member template won't be able to
access the (unconditionally) private _C_grow(). I think HP aCC
on PA-RISC actually uses this block to work around a compiler
bug (see http://issues.apache.org/jira/browse/STDCXX-271).
Martin
>
>
> Index: string
> ===================================================================
> --- string (revision 585687)
> +++ string (working copy)
> @@ -521,6 +521,8 @@
>
> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>
> +private:
> +
> size_type _C_grow (size_type, size_type) const;
>
> public:
>
>
>
>
>
>> -----Original Message-----
>> From: Martin Sebor [mailto:sebor@roguewave.com]
>> Sent: Wednesday, October 17, 2007 3:13 PM
>> To: stdcxx-dev@incubator.apache.org
>> Subject: [PATCH] std::string::_C_grow() 4.2 incompatibility
>> (was: Re: difference in exported symbols between 4.1.3 and
>> 4.2.0 15d dll's (MSVC 7.1))
>>
>> Martin Sebor wrote:
>>> Some more wisdom that I had snipped previously:
>>>
>>> Travis Vitek wrote:
>>> [...]
>>> >
>>> > Note that the basic_string<>::_C_grow() method is not
>> accessed by any
>>> > inline member functions, but it still causes problems. I
>> don't think we
>>> > can skip fixing these problems just because a function
>> appears to not be
>>> > accessable to user code.
>>> >
>>>
>>> Ah, I know why! Because it's called from a member template which
>>> gets instantiated in the test.
>>>
>>> So we need to add string::_C_grow() to the list of symbols to
>>> fix in 4.2.0.
>> Travis, can you try this patch?
>>
>> Index: include/string
>> ===================================================================
>> --- include/string (revision 585584)
>> +++ include/string (working copy)
>> @@ -504,8 +504,6 @@
>> return _C_make_iter (_C_data + __pos1);
>> }
>>
>> -public:
>> -
>> #ifndef _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>
>> template <class _InputIter>
>> @@ -519,6 +517,8 @@
>> return __rw_replace_aux (*this, __first1, __last1, __first2,
>> __last2);
>> }
>>
>> +public:
>> +
>> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>>
>> size_type _C_grow (size_type, size_type) const;
>>
>>
RE: [PATCH] std::string::_C_grow() 4.2 incompatibility (was: Re: difference in exported symbols between 4.1.3 and 4.2.0 15d dll's (MSVC 7.1))
Posted by Travis Vitek <tv...@roguewave.com>.
Martin,
That patch won't work. The _C_grow needs to be made private. It was
private in 4.1.3, but is public in 4.2. IMO it should always be private
and it was an oversight that under some conditions it could be made
public.
I have tested the following patch.
Index: string
===================================================================
--- string (revision 585687)
+++ string (working copy)
@@ -521,6 +521,8 @@
#endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
+private:
+
size_type _C_grow (size_type, size_type) const;
public:
>-----Original Message-----
>From: Martin Sebor [mailto:sebor@roguewave.com]
>Sent: Wednesday, October 17, 2007 3:13 PM
>To: stdcxx-dev@incubator.apache.org
>Subject: [PATCH] std::string::_C_grow() 4.2 incompatibility
>(was: Re: difference in exported symbols between 4.1.3 and
>4.2.0 15d dll's (MSVC 7.1))
>
>Martin Sebor wrote:
>> Some more wisdom that I had snipped previously:
>>
>> Travis Vitek wrote:
>> [...]
>> >
>> > Note that the basic_string<>::_C_grow() method is not
>accessed by any
>> > inline member functions, but it still causes problems. I
>don't think we
>> > can skip fixing these problems just because a function
>appears to not be
>> > accessable to user code.
>> >
>>
>> Ah, I know why! Because it's called from a member template which
>> gets instantiated in the test.
>>
>> So we need to add string::_C_grow() to the list of symbols to
>> fix in 4.2.0.
>
>Travis, can you try this patch?
>
>Index: include/string
>===================================================================
>--- include/string (revision 585584)
>+++ include/string (working copy)
>@@ -504,8 +504,6 @@
> return _C_make_iter (_C_data + __pos1);
> }
>
>-public:
>-
> #ifndef _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>
> template <class _InputIter>
>@@ -519,6 +517,8 @@
> return __rw_replace_aux (*this, __first1, __last1, __first2,
>__last2);
> }
>
>+public:
>+
> #endif // _RWSTD_NO_STRING_OUTLINED_MEMBER_TEMPLATES
>
> size_type _C_grow (size_type, size_type) const;
>
>