incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r678913 - in /stdcxx/branches/4.3.x: ./ etc/config/src/ examples/include/ include/ include/loc/ include/rw/ src/ tests/containers/ tests/localization/ tests/strings/ tests/utilities/
Date Wed, 23 Jul 2008 16:29:50 GMT
Eric Lemings wrote:
>  
> 
>> -----Original Message-----
>> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
>> Sent: Tuesday, July 22, 2008 4:31 PM
>> To: dev@stdcxx.apache.org
>> Subject: Re: svn commit: r678913 - in /stdcxx/branches/4.3.x: 
>> ./ etc/config/src/ examples/include/ include/ include/loc/ 
>> include/rw/ src/ tests/containers/ tests/localization/ 
>> tests/strings/ tests/utilities/
>>
> ...
>> ==============================================================
>> ================
>>> --- stdcxx/branches/4.3.x/include/deque.cc (original)
>>> +++ stdcxx/branches/4.3.x/include/deque.cc Tue Jul 22 14:24:01 2008
>>> @@ -518,8 +518,6 @@
>>>  }
>>>  
>>>  
>>> -#ifndef _RWSTD_NO_MEMBER_TEMPLATES
>>> -
>>>  template <class _TypeT, class _Allocator>
>>>  template <class _InputIter>
>>>  void deque<_TypeT, _Allocator>::
>>> @@ -529,18 +527,6 @@
>>>  
>>>      deque* const __self = this;
>> And here we should be able to do away with the __self hack. This
>> hack, btw., is probably used in other containers (string comes
>> to mind, although it doesn't look to me like your patch removes
>> this cruft from string), so we should review and clean those up
>> as well. Otherwise these strange looking vestiges will leave
>> people wondering what the heck we're doing.
> 
> Since this change is not related to STDCXX-978 and involves other
> containers not affected by this issue, we should create a new
> issue and do this cleanup as part of that issue, yes?

I believe it is directly related. The hack is part of the
workaround for _RWSTD_NO_MEMBER_TEMPLATES (there shouldn't
be any other places where we use it).

> 
> ...
>>> +
>>>      static void swap(reference __x, reference __y);
>>> +
>>>      void flip ();
>>> +
>>>      void clear()
>> These are good changes but in the future please resist the urge
>> to improve formatting in the same patch as where you're making
>> substantive changes.
> 
> I agree in principle but for very small changes like this I probably
> wouldn't bother going to the trouble of doing this separately.  :)

There are good reasons to keep formatting changes out of
unrelated patches. Besides those mentioned on our page
below, it makes reviewing big patches much easier and more
reliable.

   http://stdcxx.apache.org/bugs.html#patch_format

Thanks
Martin

> 
> I'm incorporating all other recommendations.
> 
> Thanks,
> Brad.


Mime
View raw message