Re: [eigen] StdVector

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Thank you!

I hope you feel as good about it as I do.

Cheers,
Benoit

2009/4/21 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
> OK, change done. (but I still have to update the documentation)
>
> Cheers,
> Gael.
>
> On Tue, Apr 21, 2009 at 10:27 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> 2009/4/21 Markus Moll <markus.moll@xxxxxxxxxxxxxxxx>:
>>> Hi
>>>
>>> On Tuesday 21 April 2009 14:25:48 Gael Guennebaud wrote:
>>>> On Tue, Apr 21, 2009 at 2:08 PM, Markus Moll
>>>> > Sort of. The linker error is obviously gone, but the vector interface is
>>>> > still incorrect (it's at least missing a templated constructor from an
>>>> > iterator range, which makes existing code fail to compile), and to me it
>>>> > still feels very wrong to manipulate std::vector. I like the idea of
>>>> > forcing the user to use aligned_allocator _a lot_ better.
>>>>
>>>> I fixed that issue 5 mn ago !
>>>
>>> Great.
>>>
>>> I have attached another patch that
>>> a) makes vector(size_type, const value_type& = value_type()) explicit and
>>> accept an additional const allocator_type&
>>> b) changes the __x, __a, __something names and _EIGEN_WORKAROUND_... that are
>>> reserved names to x, a, something and EIGEN_WORKAROUND...
>>>
>>> However, I haven't tested the patch so there could be some typos (I think
>>> not)...
>>>
>>>> Note that even if we enforce the user to use aligned_allocator, we
>>>> still have to reimplement the tricky resize functions that is the main
>>>> limitation because it depends on the platform
>>>
>>> Of course.
>>>
>>>> So basically, the only difference is that we would avoid the ugly
>>>> #define vector std_vector when including <vector>. But I don't think
>>>> that's a good reason to not keep the current solution which is simpler
>>>> to use.
>>>>
>>>> > The question remains: is that worth it? And why only for std::vector?
>>>> > (leaving the resize problem aside)
>>>>
>>>> because other stl types do not exhibit the "bug" of the resize
>>>> function. So, for other types just use aligned_allocator.
>>>
>>> That's exactly the point I was trying to make. With all other containers, the
>>> user has to "just use aligned_allocator", but for std::vector we try to avoid
>>> that requirement at all costs?
>>
>> In my humble opinion... bingo!
>>
>> Since we're requiring users to explicitly use aligned_allocator for
>> other containers, there's little benefit in avoiding that for
>> std::vector. The documentation doesn't get any simpler, and we break
>> the rule of "you're not annoyed by what you don't use" since our
>> std::vector specialization gets used for all types, not just Eigen
>> types.
>>
>> So I'm very impressed by the latest StdVector but I can't help
>> thinking that this is going to remain a source of problems -- there's
>> already a good deal of MSVC-specific code. I think we'd be better off
>> switching right away to the idea that you (Gael) had proposed -- to
>> only specialize for Eigen::aligned_allocator. In my opinion that was
>> the right idea.
>>
>> Cheers,
>> Benoit
>>
>>
>>
>
>
>