SmallVector::insert() fix

Jean-Luc Duprat jduprat at apple.com
Mon Jun 10 08:55:24 PDT 2013


I strongly agree that we've reached the point where SmallVector should just use std::vector once it outgrows the pre-allocated space.
Our current implementation is buggy, and will always be more work to maintain than reusing libc++.

We need consensus before I, or anyone else, goes off and writes a bunch of code.  The current implementation is buggy and needs to be fixed.  I sent out the initial email several weeks ago and people got bit by this issue since then.  We could fix it and wait for C++11 adoption in the LLVM codebase, or move to one of Howard's proposal.  

Avoiding to modify the API is probably a worthy goal, as there are a large number of users:
$ grep -iIrn SmallVector lib/|wc -l
    3919


JL


On Jun 7, 2013, at 13:59 , Michael Ilseman <milseman at apple.com> wrote:

> 
> On Jun 7, 2013, at 10:23 AM, Howard Hinnant <hhinnant at apple.com> wrote:
> 
>> On Jun 7, 2013, at 1:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>>>> Using C++11 template aliases you could even implement llvm::SmallVector<T, N> as a std::vector<T, llvm::stack_allocator<T, N>>
>>> 
>>> This would be slightly problematic - is there anything we could do for
>>> C++98 compatibility that would keep the terse naming we already have?
>> 
>> It's ugly:
>> 
>> template <class T, size_t N>
>> struct SmallVector
>> {
>>   typedef std::vector<T, stack_allocator<T, N> > type;
>> };
>> 
>> SmallVector<T, N> has to become SmallVector<T, N>::type.  If T is a dependent template parameter, add a typename in front of SmallVector.
>> 
>> The only other option I see (without template aliases) is:
>> 
>> template <class T, size_t N>
>> class SmallVector
>> {
>>   std::vector<T, stack_allocator<T, N> > data_;
>> public:
>>   // Replicate the interface here
>> };
>> 
> 
> The second option allows us to keep our asserts on access.
> 
>> C++11 is worth migrating to. :-)
>> 
> 
> Yup, I'm looking forward to that day.
> 
>> Howard
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list