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: [PATCH] Fix of STDCXX-268, STDCXX-331
Date Wed, 14 Feb 2007 00:46:50 GMT
Farid Zaripov wrote:
>> -----Original Message-----
>> From: Martin Sebor [mailto:sebor@roguewave.com] 
>> Sent: Thursday, February 08, 2007 2:36 AM
>> To: stdcxx-dev@incubator.apache.org
>> Subject: Re: [PATCH] Fix of STDCXX-268, STDCXX-331
>>
>> Farid Zaripov wrote:
>>>   Attached is a proposed patch for fix the bugs STDCXX-268 
>> and STDCXX-331.
>>
>> If the bodies of the functions are the same wouldn't be 
>> better to call one from the other rather than introducing the macro?
>> (The macro makes it impossible to step through the functions 
>> in most debuggers. I know we have macros in there already but 
>> we don't need to make things worse by adding more :)
>   I agree that it would be better.
> 
>> Or are we invoking the macro with arguments of different 
>> types each time (Iterator, pointer, const_pointer) because 
>> we're working around the lack of member templates?
>   Yes. If _RWSTD_NO_INLINE_MEMBER_TEMPLATES macro
> is defined then the range form of the method insert is present twice
> with types const_pointer and const_iterator.
> 
>>  (We should discuss and decide if we want to keep these workarounds or
> if 
>> it's time to get rid of them and assume that the compiler is
> reasonably modern).
>   I don't have the compiler without supporting the inline mebmer
> templates.
> We need to make the list of the compilers without supporting the inline
> member
> templates and then decide.

I don't know of any C++ compiler that's in widespread use
that doesn't support member templates. Even MSVC 6 handles
them (it just doesn't grok member templates defined outside
the body of the enclosing class).

So unless someone objects (I assume you're fine with it) I
propose we ditch the _RWSTD_NO_INLINE_MEMBER_TEMPLATES config
macro and all the workarounds for it. We should do a clean
sweep through the library to remove it rather than removing
it piecemeal.

We can either remove the macro first and then commit a cleaner
version of your patch w/o or we can do it the other way around.
Do you have a preference? (If the latter we should probably
open a Jira task as a reminder.)

> 
>> Btw., I wonder if we could simplify (optimize) this code so 
>> as to call erase(__start, __it) in the catch block instead of 
>> looping (the idea is that the range form of erase() might be 
>> more efficient than calling the single form repeatedly):
> 
>   Yes, but to get __start we need use loop or std::advance, because
> __start = __it - __n.
> 
>   iterator __start = __it;
>   std::advance (__start, -n);
>   erase (__start, __it);
> 
>   Another approach is to remember the iterator after first successful
> insert (__it, *__first)
> operation.
> 
>   What do you prefer?

Definitely the latter. Looping would be expensive.

Martin

Mime
View raw message