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