Re: [eigen] StdVector

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

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