[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
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?
I'm perfectly ok with specializing std::vector for aligned_allocator and
implementing a different resize function (although not being allowed by C++98,
it is _very_ clear that this is actually a fix rather than a bug). I don't
like modifying std::vector for all other types together with renaming it by
#undef/#define trickery as
1) this is not allowed by the current (and afaik also the upcoming) C++
standard (while the other solution is allowed at least by the upcoming
standard and actually ok with the current standard as it merely fixes a DR)
2) IIRC, for the above reason, an implementation is free to ignore the
#defines and might just provide a fixed set of definitions when it sees an
#include <somestdheader>. Worse, it might even preinclude all std headers even
when they are not #included explicitly
Markus
--
PGP key on www.esat.kuleuven.be/~mmoll/public_key.pgp
Fingerprint is
90C4 B47D 1A00 5AC1 9147 3197 EDA7 1E0E 99E4 9EDB
diff -aur eigen2/Eigen/StdVector eigen2-mm/Eigen/StdVector
--- eigen2/Eigen/StdVector 2009-04-21 14:59:24.716486105 +0200
+++ eigen2-mm/Eigen/StdVector 2009-04-21 15:34:35.591777364 +0200
@@ -43,7 +43,7 @@
// sometimes, MSVC detects, at compile time, that the argument x
// in std::vector::resize(size_t s,T x) won't be aligned and generate an error
// even if this function is never called. Whence this little wrapper.
- #define _EIGEN_WORKAROUND_MSVC_STD_VECTOR(T) Eigen::ei_workaround_msvc_std_vector<T>
+ #define EIGEN_WORKAROUND_MSVC_STD_VECTOR(T) Eigen::ei_workaround_msvc_std_vector<T>
template<typename T> struct ei_workaround_msvc_std_vector : public T
{
inline ei_workaround_msvc_std_vector() : T() {}
@@ -59,7 +59,7 @@
#else
- #define _EIGEN_WORKAROUND_MSVC_STD_VECTOR(T) T
+ #define EIGEN_WORKAROUND_MSVC_STD_VECTOR(T) T
#endif
@@ -73,15 +73,15 @@
typedef typename vector_base::allocator_type allocator_type; \
typedef typename vector_base::size_type size_type; \
typedef typename vector_base::iterator iterator; \
- explicit VECTOR(const allocator_type& __a = allocator_type()) : vector_base(__a) {} \
+ explicit VECTOR(const allocator_type& a = allocator_type()) : vector_base(a) {} \
template<typename InputIterator> \
- VECTOR(InputIterator first, InputIterator last, const allocator_type& __a = allocator_type()) \
- : vector_base(first, last, __a) {} \
+ VECTOR(InputIterator first, InputIterator last, const allocator_type& a = allocator_type()) \
+ : vector_base(first, last, a) {} \
VECTOR(const VECTOR& c) : vector_base(c) {} \
- VECTOR(size_type num, const value_type& val = value_type()) : vector_base(num, val) {} \
+ explicit VECTOR(size_type num, const value_type& val = value_type(), const allocator_type& a = allocator_type()) : vector_base(num, val, a) {} \
VECTOR(iterator start, iterator end) : vector_base(start, end) {} \
- VECTOR& operator=(const VECTOR& __x) { \
- vector_base::operator=(__x); \
+ VECTOR& operator=(const VECTOR& x) { \
+ vector_base::operator=(x); \
return *this; \
}
@@ -98,41 +98,41 @@
template<typename T,typename DummyAlloc>
class ei_vector<T,DummyAlloc,true>
- : public std::std_vector<_EIGEN_WORKAROUND_MSVC_STD_VECTOR(T),
- Eigen::aligned_allocator<_EIGEN_WORKAROUND_MSVC_STD_VECTOR(T)> >
+ : public std::std_vector<EIGEN_WORKAROUND_MSVC_STD_VECTOR(T),
+ Eigen::aligned_allocator<EIGEN_WORKAROUND_MSVC_STD_VECTOR(T)> >
{
- typedef std_vector<_EIGEN_WORKAROUND_MSVC_STD_VECTOR(T),
- Eigen::aligned_allocator<_EIGEN_WORKAROUND_MSVC_STD_VECTOR(T)> > vector_base;
+ typedef std_vector<EIGEN_WORKAROUND_MSVC_STD_VECTOR(T),
+ Eigen::aligned_allocator<EIGEN_WORKAROUND_MSVC_STD_VECTOR(T)> > vector_base;
EIGEN_STD_VECTOR_SPECIALIZATION_BODY(ei_vector)
- void resize(size_type __new_size)
- { resize(__new_size, T()); }
+ void resize(size_type new_size)
+ { resize(new_size, T()); }
#if defined(_VECTOR_)
// workaround MSVC std::vector implementation
- void resize(size_type __new_size, const value_type& __x)
+ void resize(size_type new_size, const value_type& x)
{
- if (vector_base::size() < __new_size)
- vector_base::_Insert_n(vector_base::end(), __new_size - vector_base::size(), __x);
- else if (__new_size < vector_base::size())
- vector_base::erase(vector_base::begin() + __new_size, vector_base::end());
+ if (vector_base::size() < new_size)
+ vector_base::_Insert_n(vector_base::end(), new_size - vector_base::size(), x);
+ else if (new_size < vector_base::size())
+ vector_base::erase(vector_base::begin() + new_size, vector_base::end());
}
- void push_back(const value_type& __x)
- { vector_base::push_back(__x); }
- iterator insert(iterator __position, const value_type& __x)
- { return vector_base::insert(__position,__x); }
- iterator insert(iterator __position, size_type __n, const value_type& __x)
- { return vector_base::insert(__position, __n, __x); }
+ void push_back(const value_type& x)
+ { vector_base::push_back(x); }
+ iterator insert(iterator position, const value_type& x)
+ { return vector_base::insert(position,x); }
+ iterator insert(iterator position, size_type n, const value_type& x)
+ { return vector_base::insert(position, n, x); }
#elif defined(_GLIBCXX_VECTOR) && EIGEN_GNUC_AT_LEAST(4,1)
// workaround GCC std::vector implementation
// Note that before gcc-4.1 we already have: std::vector::resize(size_type,const T&),
// no no need to workaround !
- void resize(size_type __new_size, const value_type& __x)
+ void resize(size_type new_size, const value_type& x)
{
- if (__new_size < vector_base::size())
- vector_base::_M_erase_at_end(this->_M_impl._M_start + __new_size);
+ if (new_size < vector_base::size())
+ vector_base::_M_erase_at_end(this->_M_impl._M_start + new_size);
else
- vector_base::insert(vector_base::end(), __new_size - vector_base::size(), __x);
+ vector_base::insert(vector_base::end(), new_size - vector_base::size(), x);
}
#else
using vector_base::resize;