Skip to content

Commit

Permalink
Remove <memory> dependency
Browse files Browse the repository at this point in the history
  • Loading branch information
vitaut committed Jan 12, 2024
1 parent 3c96084 commit 872635d
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 29 deletions.
8 changes: 3 additions & 5 deletions include/fmt/format.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
#include <cstring> // std::memcpy
#include <initializer_list> // std::initializer_list
#include <limits> // std::numeric_limits
#include <memory> // std::allocator_traits
#include <stdexcept> // std::runtime_error
#include <string> // std::string
#include <system_error> // std::system_error
Expand Down Expand Up @@ -907,17 +906,16 @@ class basic_memory_buffer : public detail::buffer<T> {
static FMT_CONSTEXPR20 void grow(detail::buffer<T>& buf, size_t size) {
detail::abort_fuzzing_if(size > 5000);
auto& self = static_cast<basic_memory_buffer&>(buf);
const size_t max_size =
std::allocator_traits<Allocator>::max_size(self.alloc_);
constexpr size_t max_size =
detail::max_value<typename Allocator::size_type>() / sizeof(T);

This comment has been minimized.

Copy link
@phprus

phprus Jan 12, 2024

Contributor

This is non equal changes.
An Allocator can support allocate only less memory than the maximum size of the type.
The use of std::allocator_traits has been added for the case when Allocator::max_size is less than max_value<typename Allocator::size_type>.

We can use SFINAE to detect Allocator::max_size() or use max_value. See: https://en.cppreference.com/w/cpp/memory/allocator_traits/max_size

This comment has been minimized.

Copy link
@phprus

phprus Jan 12, 2024

Contributor

The <memory> header file and std::allocator_traits are used in the std::string implementation (C++ standard).
This change will not reduce dependencies.

See Member types (https://en.cppreference.com/w/cpp/string/basic_string) and noexcept specification (https://en.cppreference.com/w/cpp/string/basic_string/swap).

This comment has been minimized.

Copy link
@vitaut

vitaut Jan 12, 2024

Author Contributor

We can use SFINAE to detect Allocator::max_size() or use max_value

Good idea. I haven't though about detecting max_size.

This comment has been minimized.

Copy link
@vitaut

vitaut Jan 12, 2024

Author Contributor

<string> doesn't have to include <memory> and at least libc++ manages dependencies in a much more fine-grained fashion.

This comment has been minimized.

Copy link
@vitaut

vitaut Jan 12, 2024

Author Contributor

Ah, you mean we could rely on transitive include. I guess we could try that.

This comment has been minimized.

Copy link
@phprus

phprus Jan 12, 2024

Contributor

Except gcc 4.8. Before C++11 std::basic_string does not require std::allocator_traits.
Transitive includes does not help

size_t old_capacity = buf.capacity();
size_t new_capacity = old_capacity + old_capacity / 2;
if (size > new_capacity)
new_capacity = size;
else if (new_capacity > max_size)
new_capacity = size > max_size ? size : max_size;
T* old_data = buf.data();
T* new_data =
std::allocator_traits<Allocator>::allocate(self.alloc_, new_capacity);
T* new_data = self.alloc_.allocate(new_capacity);
// Suppress a bogus -Wstringop-overflow in gcc 13.1 (#3481).
detail::assume(buf.size() <= new_capacity);
// The following code doesn't throw, so the raw pointer above doesn't leak.
Expand Down
4 changes: 1 addition & 3 deletions include/fmt/ranges.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,7 @@ struct formatter<Tuple, Char,

template <typename T, typename Char> struct is_range {
static constexpr const bool value =
detail::is_range_<T>::value && !detail::has_to_string_view<T>::value &&
!std::is_convertible<T, std::basic_string<Char>>::value &&
!std::is_convertible<T, detail::std_string_view<Char>>::value;
detail::is_range_<T>::value && !detail::has_to_string_view<T>::value;
};

namespace detail {
Expand Down
33 changes: 13 additions & 20 deletions test/format-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,37 +413,30 @@ TEST(memory_buffer_test, exception_in_deallocate) {
EXPECT_CALL(alloc, deallocate(&mem2[0], 2 * size));
}

template <typename Allocator, size_t MaxSize>
class max_size_allocator : public Allocator {
class smol_allocator : public std::allocator<char> {
public:
using typename Allocator::value_type;
size_t max_size() const noexcept { return MaxSize; }
value_type* allocate(size_t n) {
if (n > max_size()) {
using size_type = unsigned char;

auto allocate(size_t n) -> value_type* {
if (n > fmt::detail::max_value<size_type>())
throw std::length_error("size > max_size");
}
return std::allocator_traits<Allocator>::allocate(
*static_cast<Allocator*>(this), n);
return std::allocator<char>::allocate(n);
}
void deallocate(value_type* p, size_t n) {
std::allocator_traits<Allocator>::deallocate(*static_cast<Allocator*>(this),
p, n);
std::allocator<char>::deallocate(p, n);
}
};

TEST(memory_buffer_test, max_size_allocator) {
// 160 = 128 + 32
using test_allocator = max_size_allocator<std::allocator<char>, 160>;
basic_memory_buffer<char, 10, test_allocator> buffer;
buffer.resize(128);
// new_capacity = 128 + 128/2 = 192 > 160
buffer.resize(160); // Shouldn't throw.
basic_memory_buffer<char, 10, smol_allocator> buffer;
buffer.resize(200);
// new_capacity = 200 + 200/2 = 300 > 256
buffer.resize(255); // Shouldn't throw.
}

TEST(memory_buffer_test, max_size_allocator_overflow) {
using test_allocator = max_size_allocator<std::allocator<char>, 160>;
basic_memory_buffer<char, 10, test_allocator> buffer;
EXPECT_THROW(buffer.resize(161), std::exception);
basic_memory_buffer<char, 10, smol_allocator> buffer;
EXPECT_THROW(buffer.resize(256), std::exception);
}

TEST(format_test, exception_from_lib) {
Expand Down
6 changes: 5 additions & 1 deletion test/mock-allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@

template <typename T> class mock_allocator {
public:
using value_type = T;
using size_type = size_t;

mock_allocator() {}
mock_allocator(const mock_allocator&) {}
using value_type = T;

MOCK_METHOD(T*, allocate, (size_t));
MOCK_METHOD(void, deallocate, (T*, size_t));
};
Expand All @@ -35,6 +38,7 @@ template <typename Allocator> class allocator_ref {

public:
using value_type = typename Allocator::value_type;
using size_type = typename Allocator::size_type;

explicit allocator_ref(Allocator* alloc = nullptr) : alloc_(alloc) {}

Expand Down

0 comments on commit 872635d

Please sign in to comment.