Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocate member variables with custom allocator #76

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

quantumwizard
Copy link

member std::vector variables should use template parameter Allocator instead of default std::allocator

Since a custome Allocator can by provided, I think it should be used with the member std::vector variables
@mclow
Copy link
Contributor

mclow commented May 29, 2020

This shouldn't work; because std::vector requires it's allocator's value_type to match the value_type of the vector. libc++, for example, will static_assert to check this.

@quantumwizard
Copy link
Author

@mclow I made changes to rebind the allocator for the correct types

@codecov
Copy link

codecov bot commented May 29, 2020

Codecov Report

Merging #76 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop      #76   +/-   ##
========================================
  Coverage    64.33%   64.33%           
========================================
  Files           10       10           
  Lines          816      816           
  Branches       290      290           
========================================
  Hits           525      525           
  Misses         125      125           
  Partials       166      166           
Impacted Files Coverage Δ
include/boost/format/format_class.hpp 87.50% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5884c3d...a561501. Read the comment docs.

@mclow
Copy link
Contributor

mclow commented May 29, 2020

That's better. I will give this a try (and probably commit it), but this is not even close to right. (the code, not the patch).

There's no provision for actually supplying an allocator to this class. It's all "default construct" everywhere. This works great for stateless allocators, but not so well for stateful ones.

include/boost/format/format_class.hpp Outdated Show resolved Hide resolved
using boost::allocator_rebind so it will work with bost c++17 deprecation of std::allocator::rebind
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants