Reusable storage for array of objects V3

Question

Here is a second follow up on Reusable storage for array of objects and Reusable storage for array of objects V2, taking into account the provided answers.
The following code should be compliant at least with gcc, clang, msvc and for C++14 and beyond.

#include <cstddef>
#include <cstdint>
#include <functional>
#include <iostream>
#include <limits>
#include <new>
#include <type_traits>
// Object constructible from a double
// forcing alignment
struct alignas(16) SFloat {
 float val = 0.f;
 SFloat() = default;
 SFloat(const double v) : val(static_cast<float>(v)){};
 SFloat& operator=(SFloat&& f) {
 val = f.val;
 return *this;
 }
 ~SFloat() = default;
};
// Serialization of Float objects
std::ostream& operator<<(std::ostream& o, SFloat const& f) {
 return o << f.val;
}
// type-punning reusable buffer
// holds a non-typed buffer (actually a char*) that can be used to store any
// types, according to user needs
class Buffer {
 private:
 // destructing functor storage
 // required to call the correct object destructors
 std::function<void(Buffer*)> Destructors = [](Buffer*) noexcept {};
 // buffer address
 unsigned char* Storage = nullptr;
 // aligned buffer address
 unsigned char* AlignedStorage = nullptr;
 // number of stored elements
 std::size_t CountOfStoredObjects = 0;
 // buffer size in bytes
 std::size_t StorageSizeInBytes = 0;
 // computes the smallest positive offset in bytes that must be applied to
 // Storage in order to have alignment a Alignment is supposed to be a valid
 // alignment
 std::size_t OffsetForAlignment(unsigned char const* const Ptr,
 std::size_t Alignment) noexcept {
 std::size_t Res = static_cast<std::size_t>(
 reinterpret_cast<std::uintptr_t>(Ptr) % Alignment);
 if (Res) {
 return Alignment - Res;
 } else {
 return 0;
 }
 }
 public:
 // allocates a char buffer large enough for Counts object of type T and
 // default-construct them
 template <typename T>
 T* DefaultAllocate(const std::size_t Count) {
 if (Count > (std::numeric_limits<std::size_t>::max() - alignof(T)) /
 sizeof(T)) {
 throw std::bad_alloc();
 }
 // Destroy previously stored objects
 Destructors(this);
 std::size_t RequiredSize = sizeof(T) * Count + alignof(T);
 if (StorageSizeInBytes < RequiredSize) {
 // Creating a storage of RequiredSize bytes
 unsigned char* NewStorage = reinterpret_cast<unsigned char*>(
 std::realloc(Storage, RequiredSize));
 if (NewStorage == nullptr) {
 Destructors = [](Buffer*) {};
 throw std::bad_alloc();
 }
 Storage = NewStorage;
 StorageSizeInBytes = RequiredSize;
 }
 AlignedStorage = Storage + OffsetForAlignment(Storage, alignof(T));
 // initializing an dynamic array of T on the storage
 T* tmp = ::new (Storage) T[Count];
 // update nb of objects
 CountOfStoredObjects = Count;
 // create destructors functor
 Destructors = [](Buffer* pobj) noexcept {
 T* ToDestruct = reinterpret_cast<T*>(pobj->AlignedStorage);
 // Delete elements in reverse order of creation
 while (pobj->CountOfStoredObjects > 0) {
 --(pobj->CountOfStoredObjects);
 ToDestruct[(pobj->CountOfStoredObjects)].~T();
 }
 ::operator delete[](ToDestruct, ToDestruct);
 };
 return tmp;
 }
 ~Buffer() noexcept {
 // Ending objects lifetime
 Destructors(this);
 // Releasing storage
 std::free(Storage);
 }
};
int main() {
 constexpr std::size_t N0 = 7;
 constexpr std::size_t N1 = 3;
 Buffer B;
 std::cout << "Test on SomeClass\n";
 SFloat* PSFloat = B.DefaultAllocate<SFloat>(N0);
 PSFloat[0] = 3.14;
 *(PSFloat + 1) = 31.4;
 PSFloat[2] = 314.;
 std::cout << PSFloat[0] << '\n';
 std::cout << PSFloat[1] << '\n';
 std::cout << *(PSFloat + 2) << '\n';
 std::cout << "Test on float\n";
 // reallocating, possibly using existing storage, for a different type and
 // size
 float* PFloat = B.DefaultAllocate<float>(N1);
 PFloat[0] = 3.14f;
 *(PFloat + 1) = 31.4f;
 PFloat[2] = 314.f;
 std::cout << PFloat[0] << '\n';
 std::cout << PFloat[1] << '\n';
 std::cout << *(PFloat + 2) << '\n';
 return 0;
}

Live

Answer 1

Incorrect pointer used to construct objects

You are constructing the objects using the non-aligned pointer.

Use the pointer returned from new to destruct the objects

While you are using the correctly aligned pointer to destruct the objects, this pointer was created before there were any valid objects in the allocated memory. If you want to be pedantic, then this pointer might still not be valid even after new has been called. Instead, you should use a pointer derived from tmp to destruct the objects.

Think about the state of a Buffer object if new throws

You have the same issue as before with reallocation failures, except now you just moved it a bit. You really have to think about what can go wrong in each line of code, and how that can leave a Buffer object in an incorrect state. Consider the value of all the member variables at that point.

This also brings me to:

Write a test suite

Write an extensive test suite to cover all the possible ways your class can be used, all the corner cases (like creating a Buffer but never assigning anything to it), some types with huge alignment restrictions, and also how things can go wrong (replace the memory allocator with something that can fail). Use sanitizers, static analysis tools and code coverage tools to get more confidence in the correctness of your code.

Minor issues

Some small things that were already pointed out in the earlier reviews:

• Missing <cstdlib>
• Naming: short variable names (val, o, f), inconsistent style (tmp and pobj when everything else is capitalized).

• Oersted: Oups dumb mistake for the first point. For the second one, as I pointed in a comment for the v2 version, as far as I understand, one of the few guarantee given for the array placement new is that it returns the same pointer as on input: en.cppreference.com/w/cpp/language/new#Placement_new or en.cppreference.com/w/cpp/memory/new/operator_new
• Oersted: Yet I will propose the following: store reinterpret_cast<unsigned char*>(Tmp) inside a new member variable that I will cast back to T * in ToDestruct. Would it be better? Btw, I'll work on my error management, I indeed missed the fact that my object construction may fail also.
• G. Sliepen: It's only "the same" in that it points to the same address. There are other qualities of a pointer that might not be the same. An obvious one is that the type of the pointer that is returned is different than the one you pass it. Another one is that the pointer returned has the property that it points to a live object, whereas the one you pass it does not. This does not matter one bit for just running the code on any normal computer, however it might be something that static analysis tools could complain about, because it's incorrect for the abstract machine that C++ implements.
• Oersted: I understand. Thus is the casting to and from unsigned char* valid?
• G. Sliepen: Yes, casting to unsigned char* and back is valid.