instead of PIMPL, use the implstore

If you can't afford the extra level of indirection, that comes with PIMPL, use the implstore! Improvements?

```
#ifndef IMPLSTORE_HPP
# define IMPLSTORE_HPP
# pragma once

#include

#include

#include

#include

template
class implstore
{
public:
implstore()
{
static_assert(sizeof(U) {},
"impl is not default constructible");
new (store_) U;

deleter_ = deleter_stub;
}

template
explicit implstore(A&& ...args)
{
static_assert(sizeof(U) (args)...);

deleter_ = deleter_stub;
}

~implstore() { this && (deleter_(this), true); }

template
implstore(implstore const& other)
{
static_assert(::std::is_copy_constructible{},
"impl is not copy constructible");
new (store_) U(*other);

deleter_ = other.deleter_;
}

template
implstore(implstore&& other)
{
static_assert(::std::is_move_constructible{},
"impl is not move constructible");

new (store_) U(*other);

deleter_ = other.deleter_;
}

template
implstore& operator=(implstore const& other)
{
static_assert(::std::is_copy_assignable{},
"impl is not copy assignable");

**this = *other;

deleter_ = other.deleter_;

return *this;
}

template
implstore& operator=(implstore&& other)
{
static_assert(::std::is_move_assignable{},
"impl is not move assignable");

**this = ::std::move(*other);

deleter_ = other.deleter_;

return *this;
}

U const* operator->() const noexcept
{
return reinterpret_cast(store_);
}

U* operator->() noexcept
{
return reinterpret_cast(store_);
}

U const* get() const noexcept
{
return reinterpret_cast(store_);
}

U* get() noexcept
{
return reinterpret_cast(store_);
}

U const& operator*() const noexcept
{
return *reinterpret_cast(store_);
}

U& operator*() noexcept
{
return *reinterpret_cast(store_);
}

explicit operator bool() const noexcept { return deleter_; }

private:
static voi

new (store_) U;

You should explicitly cast to a void* to avoid calling any user-defined allocation functions instead of placement-new.

void (*deleter_)(implstore&){};

To my eyes, this is quite ugly. It almost looks like a member function, but it's a pointer. I'd prefer

using deleter_t = void(*)(implstore&);
deleter_t deleter_ = nullptr;

But I think both the NSDMI (= nullptr or {}) and deleter_ itself is unnecessary. Since you now changed the code so that every constructor initializes the stored object, you don't even need it as a check whether or not the object is initialized. If you leave out the deleter_, you force the user of implstore to define their dtor out-of-line at a point where the type stored inside the implstore is complete. If the user doesn't do that, the error message is not very nice (destructor of incomplete type called at the point where the class using implstore is destroyed), but it saves you 8 bytes per object. The cost of the function call has to be paid anyway AFAIK.

template 
explicit implstore(A&& ...args)

Typically, such constructors are too greedy and should be restricted by SFINAE via std::is_constructible so that they only produce instantiations for arguments for which the stored object can be constructed. Otherwise, implstore will appear constructible from any argument set (std::is_constructible will always yield true).

Additionally, constructor templates and assignment-operator templates do not suppress the generation of compiler-generated copy/move constructors and assignment-operators.

~implstore() { *this && (deleter_(*this), true); }

I'd prefer

~implstore() { if(*this) deleter_(*this); }

which is shorter and (arguably) clearer. But neither is necessary with your changed code:

~implstore() { get()->~U(); }

should be sufficient.

Here's an example of how it can/needs to be used:

// MyClass.hpp
class MyClass
{
private:
    struct Impl;
    implstore impl;
public:
    MyClass();
    MyClass(MyClass const&);
    ~MyClass();
    MyClass& operator=(MyClass const&);

    void meow();
};

// MyClass.cpp
#include 
#include "MyClass.hpp"

struct MyClass::Impl
{
    void meow() { std::cout meow(); }

// other.cpp
#include "MyClass.hpp"

int main()
{
    MyClass m;
    m.meow();
}

The important part of course is the out-of-line definition of all member functions of MyClass that require Impl to be complete. This includes the destructor.

template 
implstore(implstore const& other)

.. and other constructors: I'd use constructor delegation or an initialization function to get rid of the duplicated code. Similarly,

U const* operator->() const noexcept
{
  return reinterpret_cast(store_);
}

and all other variations can use a single (private const) member function instead of repeating that cast.

alignas(::std::max_align_t) char store_[N];

Can, I think, be reduced to

typename std::aligned_storage::type store_;

But I'm not sure about the aliasing correctness in either case (especially combined with the reinterpret_casts). But that's probably a StackOverflow question.

Furthermore, I miss the publication of the template arguments. As a user, you can always get them out of the type by partial specialization, so they're not private anyway.

using value_type = U;
static constexpr ::std::size_t buffer_size = N;

And you could use those names instead of template parameters inside the class itself (arguably "nicer" and decouples the types from the template parameters).

StackExchange Code Review Q#50931, answer score: 4