Re: [eigen] StdVector

[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
>
>
>