Hotfix/allocator

[bassoy]

This is a hotfix for issue #96 which should be merged ASAP.

[jwakely]

You should really have tests that verify the code can be used with a minimal allocator that only provides the members required by a C++11 allocator (i.e. value_type, allocate, deallocate, and ==/!=). All other members should be accessed via allocator_traits to handle the case where the allocator doesn't provide them.

For testing libstdc++ I use:

  template <class Tp>
    struct SimpleAllocator
    {
      typedef Tp value_type;

      SimpleAllocator() noexcept { }

      template <class T>
        SimpleAllocator(const SimpleAllocator<T>&) { }

      Tp *allocate(std::size_t n)
      { return std::allocator<Tp>().allocate(n); }

      void deallocate(Tp *p, std::size_t n)
      { std::allocator<Tp>().deallocate(p, n); }
    };

  template <class T, class U>
    bool operator==(const SimpleAllocator<T>&, const SimpleAllocator<U>&)
    { return true; }
#if __cpp_impl_three_way_comparison < 201907L
  template <class T, class U>
    bool operator!=(const SimpleAllocator<T>&, const SimpleAllocator<U>&)
    { return false; }
#endif

[bassoy]

You should really have tests that verify the code can be used with a minimal allocator that only provides the members required by a C++11 allocator (i.e. value_type, allocate, deallocate, and ==/!=). All other members should be accessed via allocator_traits to handle the case where the allocator doesn't provide them.

Yes, the code coverage for some Boost.uBlas data types are poor.

For testing libstdc++ I use:

  template <class Tp>
    struct SimpleAllocator
    {
      typedef Tp value_type;

      SimpleAllocator() noexcept { }

      template <class T>
        SimpleAllocator(const SimpleAllocator<T>&) { }

      Tp *allocate(std::size_t n)
      { return std::allocator<Tp>().allocate(n); }

      void deallocate(Tp *p, std::size_t n)
      { std::allocator<Tp>().deallocate(p, n); }
    };

  template <class T, class U>
    bool operator==(const SimpleAllocator<T>&, const SimpleAllocator<U>&)
    { return true; }
#if __cpp_impl_three_way_comparison < 201907L
  template <class T, class U>
    bool operator!=(const SimpleAllocator<T>&, const SimpleAllocator<U>&)
    { return false; }
#endif

Thanks for providing the simple allocator. We are moving from one-/two- to multi-dimensional types (where one- or two dimensions will be special cases) making many of the current types obsoluete and therefore depcretated. Hence, our main focus will be on developing the multi-dimensional type.

[amitsingh19975]

Looks good.

[glenfe]

This PR is not good. We don't want to use __cplusplus >= 201103L, if anything we would want to use !defined(BOOST_NO_CXX11_ALLOCATOR) but instead of all of that:

Edit: On top of that, you can't assume size_type and difference_type exist for every Allocator either. The same for max_size().

Just include <boost/core/allocator_access.hpp> and use the appropriate facilities from there. They do the right thing and support minimal allocators. This also avoids having to have conditional code depending on whether allocator_traits is available or not.

[bassoy]

This PR is not good. We don't want to use __cplusplus >= 201103L, if anything we would want to use !defined(BOOST_NO_CXX11_ALLOCATOR) but instead of all of that:

Just include <boost/core/allocator_access.hpp> and use the appropriate facilities from there. They do the right thing and support minimal allocators. This also avoids having to have conditional code depending on whether allocator_traits is available or not.

Thanks @glenfe
Do we have time left for merging master this into the boost 1.74 release?

[mclow]

The deadline for putting things into the 1.74.0 release was .. yesterday.
As far as I can tell, this hasn't been tested on develop, let alone "is ready to be merged to master". Is that correct?

[glenfe]

See glenfe@937da38

I'll PR from my repository after adding some tests for minimal allocators.

[bassoy]

@glenfe, I just wanted to update the PR using boost::core::allocator_accessor. :-)
Thanks for helping out here.

[bassoy]

The deadline for putting things into the 1.74.0 release was .. yesterday.
As far as I can tell, this hasn't been tested on develop, let alone "is ready to be merged to master". Is that correct?

The unbounded_array is unit tested in AppVeyor and Travis. However, the test coverage might not be very high here.

[glenfe]

@bassoy See https://github.com/boostorg/ublas/pull/103/files .

[bassoy]

@bassoy See https://github.com/boostorg/ublas/pull/103/files .

Thanks @glenfe.
Please note that this PR now also includes the boost::allocator_access as suggested and also includes boost::numeric::ublas::forward_iterator_base while the PR #103 includes test cases and some minor modifications in vector.h and storage_sparse.hpp.

To be consistent with the latter modifications, using boost::allocator_size_type and boost::allocator_difference_type I suggest to use these types also in matrix.h and then to merge it with the master branch.

[glenfe]

PR #103 adjusts all places which use allocators, not just storage.hpp. What is it missing?

[bassoy]

PR #103 adjusts all places which use allocators, not just storage.hpp. What is it missing?

For the sake of consistency, using boost::allocator_size_type and boost::allocator_difference_type I suggest to use these types also in matrix.h.

I found three places in matrix.h

ublas/include/boost/numeric/ublas/matrix.hpp

Line 3463 in a31e5cf

typedef typename ALLOC::size_type size_type;

ublas/include/boost/numeric/ublas/matrix.hpp

Line 3938 in a31e5cf

typedef typename ALLOC::size_type size_type;

ublas/include/boost/numeric/ublas/matrix.hpp

Line 3463 in a31e5cf

typedef typename ALLOC::size_type size_type;

[glenfe]

@bassoy

For the sake of consistency, using boost::allocator_size_type and boost::allocator_difference_type I suggest to use these types also in matrix.h.

In #103 they are being used also in matrix.hpp.

[glenfe]

If you look at #103 carefully, you'll see that it modifies the following files:

• matrix.hpp
• storage.hpp
• storage_sparse.hpp
• vector.hpp

[bassoy]

@bassoy

For the sake of consistency, using boost::allocator_size_type and boost::allocator_difference_type I suggest to use these types also in matrix.h.

In #103 they are being used also in matrix.hpp.

Great. I will close this PR and switch to the PR #103