[libcxx-commits] [libcxx] [libc++][test] Fix increasing_allocator to meet `Cpp17Allocator` requirements (PR #115671)

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Mon Nov 11 13:25:48 PST 2024


ldionne wrote:

> The intention of `allocate_at_least` is that the allocator tells us how much it overallocated due to its allocation algorithm, not that it actually allocates more. If you implement the interface properly `allocate` will only hide the overallocation, not prevent it.

I agree, and I don't see how this makes the allocator more conforming than it used to be. However, I do think the code makes more sense after the patch than it did before the patch, because implementing `allocate()` using `std::allocator::allocate` makes more sense than doing so with `allocate_at_least`.


> Actually, the tests failed with MSVC STL (or MSVC, the compiler). MSVC detects size mismatch of deallocation in constant evaluation. I guess we can't reliably perform over allocation in constant evaluation.

```cpp
constexpr std::allocation_result<T*> allocate_at_least(std::size_t n);
constexpr T* allocate(std::size_t n) {
  auto res = allocate_at_least(n);
  return res.ptr;
}
constexpr void deallocate(T* p, std::size_t n) noexcept {
  // n may be wrong: it must match res.count which may not be the case if we used allocate_at_least above
  std::allocator<T>{}.deallocate(p, n);
}
```

Is that what you're saying? I would understand why the current code is a problem in that case, however http://eel.is/c++draft/allocator.members#10.1 says:

> If p is memory that was obtained by a call to allocate_at_least, let ret be the value returned and req be the value passed as the first argument to that call[.](http://eel.is/c++draft/allocator.members#10.1.sentence-1) p is equal to ret.ptr and n is a value such that req  ≤ n  ≤ ret[.](http://eel.is/c++draft/allocator.members#10.1.sentence-2)count.

In other words, I think it's supposed to be just fine to call `std::allocator::deallocate` with a `n` that is less than the count returned by a previous `allocate_at_least`.

https://github.com/llvm/llvm-project/pull/115671


More information about the libcxx-commits mailing list