3
\$\begingroup\$

Here is a follow up on Reusable storage for array of objects, 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 <new>
#include <type_traits>

template <typename T>
constexpr T* Launder(T* Ptr) {
#if __cplusplus < 201703L
    // Launder in C++<17
    return Ptr;
#else
    // Launder in C++>=17
    return std::launder(Ptr);
#endif
}

// Object constructible from a double
// forcing alignment
struct alignas(16) SFloat {
    float val = 0.f;
    SFloat() { std::cout << "Constructing a SFloat with default value\n"; };
    SFloat(const double v) : val(static_cast<float>(v)) {
        std::cout << "Constructing a SFloat from " << v << '\n';
    };
    SFloat& operator=(SFloat&& f) {
        val = f.val;
        std::cout << "Move-assigning from a SFloat " << f.val << '\n';
        return *this;
    }
    ~SFloat() { std::cout << "Destructing a SFloat holding " << val << '\n'; }
};
// 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
    // using std::function to store a copy of the lambda used
    // @1 is there a way to avoid std::function? a mere function pointer could
    // be used but it does not compile with msvc/C++14
    // @2 std::function does not support noexcept properly how to do better?
    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) {
        // Destroy previously stored objects, supposedly ends lifetime of the
        // array object that contains them
        Destructors(this);
        std::size_t RequiredSize = sizeof(T) * Count + alignof(T);
        if (StorageSizeInBytes < RequiredSize) {
            std::cout << "Requiring " << RequiredSize << " bytes of storage\n";
            // @3 creating a storage of RequiredSize bytes
            // what would be the C++17+ way of do that? std::aligned_alloc?
            Storage = reinterpret_cast<unsigned char*>(
                std::realloc(Storage, RequiredSize));
            if (Storage == nullptr) {
                throw(std::bad_alloc());
            }
            StorageSizeInBytes = RequiredSize;
            // here should do something for the case where Storage is nullptr
        }
        AlignedStorage = Storage + OffsetForAlignment(Storage, alignof(T));
        // @4 Form1 placement array new default construction: ill-defined in
        // C++14?
        // expecting starting an array object lifetime and the lifetime of
        // contained objects
        // expecting pointer arithmetic to be valid on tmp T*
        // Is the use of Launder correct?
        T* tmp = Launder(new (Storage) T[Count]);
        // @5 Form2 individually creating packed object in storage
        // expecting starting an array object lifetime and the lifetime of
        // contained objects
        // expecting pointer arithmetic to be valid on tmp T*
        // Is the use of Launder correct?
        // unsigned char* pos = AlignedStorage;
        // T* tmp = Launder(reinterpret_cast<T*>(AlignedStorage));
        // for (std::size_t i = 0; i < Count; ++i) {
        //     new (pos) T();
        //     pos += sizeof(T);
        // }

        // update nb of objects
        CountOfStoredObjects = Count;
        // create destructors functor
        // @6 supposedly ends the lifetime of the array object and of the
        // contained objects
        Destructors = [](Buffer* pobj) noexcept {
            T* ToDestruct = reinterpret_cast<T*>(pobj->AlignedStorage);
            // Delete elements in reverse order of creation
            while (pobj->CountOfStoredObjects > 0) {
                --(pobj->CountOfStoredObjects);
                // should be std::Destroy(ToDestruct[CountOfStoredObjects]) in
                // C++17 I should provide my own implementation in C++14 in
                // order to distinguish between fundamental types and other ones
                // @ how to formally en the lifetime of a fundamental objects?
                // merely rewrite on its memory location?
                ToDestruct[(pobj->CountOfStoredObjects)].~T();
            }
            // @7 How to formally end the array object lifetime?
        };
        return tmp;
    }

    // deallocate objects in buffer but not the buffer itself
    // actually useless
    // template <typename T>
    // void Deallocate() {
    //     Destructors(this);
    // }
    ~Buffer() noexcept {
        // Ending objects lifetime
        // @8 Actually not throwing and compilers are not complaining
        // should I wrap it anyway into a try-catch block? 
        Destructors(this);
        // Releasing storage
        std::free(Storage);
    }
};

int main() {
#if (__cplusplus >= 201703L)
    std::cout << "C++17\n";
#else
    std::cout << "C++14\n";
#endif
    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

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Consider using or reimplementing std::any

// holds a non-typed buffer (actually a char*) that can be used to store any
// types, according to user needs

This is exactly what std::any does, which was added in C++17. I would strongly recommend you use that, or if you need something like this in C++14 projects, then consider making your Buffer a drop-in replacement for std::any, and copy its interface.

Answers to the questions in the code

// @1 is there a way to avoid std::function? a mere function pointer could
// be used but it does not compile with msvc/C++14 

A mere function pointer works fine as far as I can tell:

void (*Destructors)(Buffer*) = [](Buffer*) noexcept {};
// @2 std::function does not support noexcept properly how to do better?

I would not worry about it. I don't think it has any impact on runtime performance as long as nothing actually throws. You can still make ~Buffer() noexcept.

// @3 creating a storage of RequiredSize bytes
// what would be the C++17+ way of do that? std::aligned_alloc?

Yes. Or from C++11 up to C++23 there is std::aligned_storage, but this might not deal properly with over-aligned types. Note also that you can use new char[RequiredSize] and delete[] to allocate the uninitialized storage, there is no need to use C's std::malloc()/std::free().

// @4 Form1 placement array new default construction: ill-defined in C++14?

I don't know why this would be ill-defined, unless T has no default constructor.

// Is the use of Launder correct?
T* tmp = Launder(new (Storage) T[Count]);

It's unnecessary to use std::launder() here: the return value of placement-new is already a valid pointer to live objects of type T. The code that is commented out under question @5 uses std::launder() for a good reason.

// @ how to formally en the lifetime of a fundamental objects?
// merely rewrite on its memory location?
ToDestruct[(pobj->CountOfStoredObjects)].~T();

That's perfectly fine for all types of objects, there is no need to use any other code for fundamental objects. The fundamental types have destructors that just do nothing.

// @7 How to formally end the array object lifetime?

Technically, I think you should call the non-allocating placement deallocation variant of operator delete[]. This is what placement-new calls when a constructor throws. However, by default this does absolutely nothing. It's only if the program provides a custom implementation of operator delete[] that you need to worry about this.

// @8 Actually not throwing and compilers are not complaining // should I wrap it anyway into a try-catch block?

No. If the call to Destructors() throws and you don't catch it, then because your function is noexcept, the exception won't be passed to the caller, but instead std::terminate() will be called.

Possible integer overflow

In this line:

std::size_t RequiredSize = sizeof(T) * Count + alignof(T);

You can have an integer overflow if the result of the calculation is larger than a std::size_t can hold. You should check for this in a safe way:

if (Count > (SIZE_MAX - alignof(T)) / sizeof(T)) {
    throw std::bad_alloc("…");
}

Incorrect handling of reallocation failure

In the previous iteration of your code, Toby Speight already pointed out that you are not handling std::realloc() returning nullptr correctly. Your fix is incorrect, and makes the problem even worse. First, if it fails, the original allocation is still valid, but you've thrown away the pointer to it, causing a memory leak. But by throwing, you now also left Destructors to point to the old destructor function which you have already used. So when your Buffer is then destructed, it will be called again, which is incorrect. Carefully reread Toby's answer for the correct way to handle this.

\$\endgroup\$
9
  • \$\begingroup\$ Very clear, thank you. Just a few comments. 1- Regarding std::any I was unsure that it reuse existing storage, but maybe that it is exactly what the emplace function does. Yet I can't find the information. Anyway, as I may need it in C++14, looking inside the implementation would be necessary. Regarding function pointer, I does not compile so far with msvc and C++14. It might be a bug. Eventually, I may use the special delete[] function though, as its new[] counterpart, it is unspecified what effect it has on objects and array lifetime. \$\endgroup\$
    – Oersted
    Commented Sep 5, 2023 at 7:19
  • \$\begingroup\$ by the way, how should I call the delete[]? I thought about operator delete[](ToDestruct,ToDestruct);, after the objects destruction loop. \$\endgroup\$
    – Oersted
    Commented Sep 5, 2023 at 7:32
  • \$\begingroup\$ I think it's specified what the effect of new[] is on the lifetime of the objects in the array, the unspecified part is if it will do anything besides that (like store the size of the array somewhere). Personally, I would consider this extremely unlikely, like having your code run on a platform where CHAR_BITS != 8, and just use it because it is the most practical thing. C++17 introduces some functions like std::unitialized_value_construct_n() which make it almost as easy as using placement-array-new. \$\endgroup\$
    – G. Sliepen
    Commented Sep 5, 2023 at 7:35
  • \$\begingroup\$ Yes, that way of calling operator delete[] is correct I think. Technically, array-placement-new can return a different pointer than the one you passed it (for example, if it prepends an array size to the actual objects), in which case you'd have to pass two different pointers to operator delete[] as well, but in that unlikely case it would break your DefaultAllocate() function anyway. \$\endgroup\$
    – G. Sliepen
    Commented Sep 5, 2023 at 7:38
  • \$\begingroup\$ If I'm not wrong, one of the very few things the standard says about array placement new is that it returns the pointer unchanged. \$\endgroup\$
    – Oersted
    Commented Sep 5, 2023 at 7:48

Not the answer you're looking for? Browse other questions tagged or ask your own question.