This test case shows the problem: #include <Eigen/Core> #include <set> struct vertex : Eigen::Vector2d { vertex(double x, double y, int16_t i = -1) : Eigen::Vector2d(x,y), index(i) {} // strict weak ordering for std::set bool operator<(const vertex& v) const { return x() != v.x() ? x() < v.x() : y() < v.y(); } int16_t index; }; int main() { std::set< vertex, std::less<vertex>, Eigen::aligned_allocator<vertex> > vertices; vertex v(1.0, 2.0); vertices.insert(v); return 0; } It doesn't compile with: clang -I... -c bug.cpp -std=c++11 -stdlib=libc++ The fix is commenting out both overloads of construct() in aligned_allocator, as indicated in the accepted answer here: http://stackoverflow.com/questions/11648202/questions-about-hinnants-stack-allocator Then, however compilation breaks with pre-11 CC++. So I guess something is wrong with construct(), but no idea what.
Which Eigen version? Is't it a duplicate of bug 455
I don't think it's a duplicate. It breaks in 3.1 and in the default branch, 3.0 works though.
The problem is that clang support for c++11 is really incomplete. Alos, commenting out both overloads does not work for me and clang: In file included from /usr/include/c++/4.2.1/set:64: /usr/include/c++/4.2.1/bits/stl_tree.h:379:22: error: no member named 'construct' in 'Eigen::aligned_allocator<vertex>' { get_allocator().construct(&__tmp->_M_value_field, __x); } ~~~~~~~~~~~~~~~ ^ /usr/include/c++/4.2.1/bits/stl_tree.h:839:24: note: in instantiation of member function 'std::_Rb_tree<vertex, vertex, std::_Identity<vertex>, std::less<vertex>, Eigen::aligned_allocator<vertex> >::_M_create_node' requested here _Link_type __z = _M_create_node(__v); ^ /usr/include/c++/4.2.1/bits/stl_tree.h:987:31: note: in instantiation of member function 'std::_Rb_tree<vertex, vertex, std::_Identity<vertex>, std::less<vertex>, Eigen::aligned_allocator<vertex> >::_M_insert' requested here return pair<iterator,bool>(_M_insert(__x, __y, __v), true); ^ /usr/include/c++/4.2.1/bits/stl_set.h:307:9: note: in instantiation of member function 'std::_Rb_tree<vertex, vertex, std::_Identity<vertex>, std::less<vertex>, Eigen::aligned_allocator<vertex> >::_M_insert_unique' requested here _M_t._M_insert_unique(__x); ^ bug_503.cpp:25:14: note: in instantiation of member function 'std::set<vertex, std::less<vertex>, Eigen::aligned_allocator<vertex> >::insert' requested here vertices.insert(v); ^ 1 error generated. (when using clang 3.1)
hm, ok it works if you also specify -stdlib=libc++
hm, now I'm puzzled. If I compile with both -std=c++11 and -stdlib=libc++, then the current code works fine. It only breaks if I remover -stdlib=libc++, but then removing the construct overloads also fails.
For me, commenting out construct() is only needed with libc++, which makes sense, since Clang's default STL implementation is not C++-11 compliant. Maybe specialzing allocatior_traits is the way to go? According to http://home.roadrunner.com/~hinnant/stack_alloc.html C++-03 allocators should work in C++11...
One cannot remove both overloads of construct. The allocator requirements were weakened in C++11: construct is now *optional*, even though there is a new overload that is *allowed*. Still, the C++11 overload is supposed to accept *any* pointer type, not just the allocator's `pointer` type. The code should still work fine like this, as allocator_traits is supposed to default to placement-new if a given call to construct is not well-formed. Why it doesn't work with certain compiler/library combinations, I can't tell for sure, but it sounds like a compiler/library bug. Since the construct overloads are optional, and the current implementations merely perform the default behaviour anyway, they can both be removed for C++11. Remember however, that I mentioned the requirements were *weakened*. In C++03, the single argument overload is still *required*. In light of that I removed only the C++11 overload and the sample code compiles and runs fine with all of the following setups: - gcc + libstdc++ @ C++03 - gcc + libstdc++ @ C++11 - clang + libstdc++ @ C++03 - clang + libstdc++ @ C++11 - clang + libc++ @ C++03 - clang + libc++ @ C++11 (commit here https://bitbucket.org/martinhofernandes/eigen/commits/450a9024838125a0569879450ffd3d512934e914) I issued pull request #33 (https://bitbucket.org/eigen/eigen/pull-request/33/fix-for-bug-503/diff) for this.
I can confirm that both... * by deleting the C++11 overload, i.e. by reversing the commit https://bitbucket.org/eigen/eigen/commits/ad274b0ede15e89e554f151e21041b7630144f1e * or by changing the overload as dictated by the standard (see http://en.cppreference.com/w/cpp/memory/allocator/construct) template <typename U, typename... Args> void construct(U* p, Args&&... args) { ::new((void*)p) T(std::forward<Args>(args)...); } the issue is remedied. Could pull request #33 be considered by any chance?
pull request accepted. I guess that in bug 455 we hit a compiler bug since I cannot reproduce the issue we had at that time.
Hmm, sadly, that pull request was kinda sorta not so good. Thing is, removing the overload throws us into the adorable hell that is the spawn of the illicit relationship between implicit conversions and overload resolution. Yes, `std::allocator_traits` will fill in whatever `construct` overloads are necessary, but in some cases it will think that the existing C++03 overload is good enough. Consider: struct foo { foo(int); foo(foo const&) = delete; }; This type has an implicit conversion from int and cannot be copy-constructed. Now suppose we call `v.emplace_back(42)` on some vector of `foo` with our allocator. `allocator_traits` check if `a.construct(42)` is well-formed for our allocator `a`, and if not, will provide its default implementation that does placement-new. But `a.construct(42)` is perfectly well-formed! `42` implicitly converts to a `foo` temporary, and a `foo` temporary matches the existing C++03 overload `construct(foo const&)`. allocator_traits will be extremely happy that for once one of those lazy allocator bastards has taken upon itself to do some work and just forward to that overload. It's a story that ends in tears: that overload performs a copy and our type is not copyable. So the C++11 overload *must exist* so that allocator_traits does the right thing and picks it over the C++03 overload. The correct implementation is: template <typename U, typename... Args> void construct( U* u, Args&&... args) { ::new( static_cast<void*>(u) ) U( std::forward<Args>( args )... ); } See PR#40 https://bitbucket.org/eigen/eigen/pull-request/40
What about making aligned_allocator inherit std::allocator so we do not have to bother about all these details and be more future proof? We probably also have to add 'using base::construct' to be sure overloads are reachable.
I finally applied the pull-request, but I'd still be interested by a simpler implementation based on std::allocator: template<class T> class aligned_allocator : public std::allocator<T> { public: typedef size_t size_type; typedef std::ptrdiff_t difference_type; typedef T* pointer; typedef const T* const_pointer; typedef T& reference; typedef const T& const_reference; typedef T value_type; template<class U> struct rebind { typedef aligned_allocator<U> other; }; aligned_allocator() : std::allocator<T>() { } aligned_allocator( const aligned_allocator& other ) : std::allocator<T>(other) { } template<class U> aligned_allocator( const aligned_allocator<U>& other ) : std::allocator<T>(other) { } ~aligned_allocator() { } pointer allocate( size_type num, const void* hint = 0 ) { EIGEN_UNUSED_VARIABLE(hint); internal::check_size_for_overflow<T>(num); return static_cast<pointer>( internal::aligned_malloc( num * sizeof(T) ) ); } void deallocate( pointer p, size_type /*num*/ ) { internal::aligned_free( p ); } }; any opinion?
I've been hitting this issue too, but in a slightly modified form. In the code sample below where std::map is used instead of std::set, PR#40 doesn't fully solve the issue. Clang (Output from Apple LLVM version 5.1 (clang-503.0.38) (based on LLVM 3.4svn)) raises compilations errors when using the current Eigen development branch. It compiles fine with gcc 4.6.4. clang -std=c++0x -stdlib=libc++ -I /usr/local/include/eigen3/ main.cpp ... In file included from /usr/local/include/eigen3/Eigen/Core:284: /usr/local/include/eigen3/Eigen/src/Core/util/Memory.h:768:16: error: static_cast from 'const vertex *' to 'void *' is not allowed ... #include <Eigen/Core> #include <map> struct vertex : Eigen::Vector2d { vertex(double x, double y, int16_t i = -1) : Eigen::Vector2d(x,y), index(i) {} // strict weak ordering for std::set bool operator<(const vertex& v) const { return x() != v.x() ? x() < v.x() : y() < v.y(); } int16_t index; }; int main() { std::map<vertex, int, std::less<vertex>, Eigen::aligned_allocator< std::pair<vertex, int> > > vertices; vertex v(1.0, 2.0); vertices[v] = 1.0; return 0; }
(In reply to comment #11) Martinho, do you have a self-contained example of your use case with class foo, because the following piece of code does not compile with gcc or clang, even without our aligned_allocator. #include <vector> struct Foo { Foo(int); Foo(Foo const&) = delete; }; int main() { std::vector<Foo> vec; vec.emplace_back(42); return 0; }
(In reply to comment #14) My proposal in comment #13 fixes your issue, though it's weird that construct is called with a const pointer. I'm trying to gather all the tricky cases we found so far before applying that change.
Apologies for my overly simplistic explanation above. The issue with the bad `construct` being called shows up with types that are not copyable, as described. The type `foo` in the example is neither movable nor copyable, since providing a deleted definition of the copy constructor inhibits the automatic generation of the move constructor, which means it won't work in most containers, but that is unrelated to the issue with the allocator. It should look as follows to be usable in vector, and still trigger the bad overload resolution. struct foo { foo(int) {} foo(foo &&) = default; foo(foo const&) = delete; };
Right. So I finally make aligned_allocator inherit std::allocator: https://bitbucket.org/eigen/eigen/commits/f5b7700fef82/ Changeset: f5b7700fef82 User: ggael Date: 2014-03-19 13:28:50 Summary: Simpler and hopefully more future-proof fix for bug 503 (aligned_allocator with c++11)
-- GitLab Migration Automatic Message -- This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/503.