2
\$\begingroup\$

Are there any issues with this class template? It is supposed to provide an easy-to-use object solution to initialize an object of class type after the declaration of the variable.

One nuisance is the implicit cast operator. In my opinion, it’s a necessary evil, otherwise the usefulness of the object is reduced, but if you have a good counter-argument or a good replacement for it, I’d like to hear it.

template<typename T>
class late_init final
{
    alignas(T) std::byte buffer[sizeof(T)];
#ifndef NDEBUG
    bool initialized = false;
#endif

public:
    ~late_init() noexcept(noexcept(reinterpret_cast<T&>(buffer).~T()))
    {
        assert(initialized);
        reinterpret_cast<T&>(buffer).~T();
    }

    late_init(const late_init&) = delete;

    explicit late_init() = default;

    template<typename... Args>
    void init(Args&&... args) noexcept(noexcept(T{ std::forward<Args>(args)... }))
    {
        assert(!initialized);
        new (buffer) T{ std::forward<Args>(args)... };
#ifndef NDEBUG
        initialized = true;
#endif
    }

    operator T& () noexcept
    {
        assert(initialized);
        return reinterpret_cast<T&>(buffer);
    }
};

Example use case requested by Martin York:

late_init<my_class> my_object;
{
    example_raii_class temp_resource{ … };
    my_object.init(…, temp_resource.something(), …);
    // temp_resource destructor runs
}
// use my_object

As of now, the example_raii_class is (varying forms of) lock objects that lock and unlock a mutex using RAII.

This class is supposed to make it into our general utils and should therefore not be overly specific.


@Davislor, I came up with needing this exactly because a class for which I needed late initialization has no default constructor.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ Code Review requires concrete code from a project, with enough code and / or context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. \$\endgroup\$
    – Mast
    Mar 6 at 14:45
  • \$\begingroup\$ How? I can’t post stuff that may fall under NDA. \$\endgroup\$
    – Bolpat
    Mar 6 at 15:28
  • 1
    \$\begingroup\$ Perhaps it's not reviewable by us then. \$\endgroup\$
    – Mast
    Mar 6 at 15:42
  • \$\begingroup\$ I reworked the question, removed unnecessary stuff, so the focus is on the late_init class. Now it is concrete code that will make it into our repo, possibly with changes if needed – which is what I hope to get to here. \$\endgroup\$
    – Bolpat
    Mar 6 at 16:19
  • 1
    \$\begingroup\$ @Davislor, you nailed it. Not every type is default-initializable, and in some of my use-cases, T has no default constructor. \$\endgroup\$
    – Bolpat
    Mar 7 at 9:33

2 Answers 2

5
\$\begingroup\$

It's very similar to std::optional

With debugging enabled, your class is basically equivalent to std::optional, as that also potentially stores a value and a flag to indicate whether a value is present. Your class is maybe a little bit smaller if debugging is disabled. If that isn't really important, consider just using std::optional. Or alternatively, have a look how std::optional is implemented to see what things you could copy from it.

Missing use of std::launder()

Merely reinterpret_casting buffer to T& is technically not legal in C++. Here are a few ways to get a legal pointer/reference to T from buffer:

  1. Using the return value of the placement-new call.
  2. Using std::launder() on the result of reinterpret_cast<>().
  3. Since C++23 you can use std::start_lifetime_as().

Missing const accessor

You should add an overload of operator T&() that is const-qualified.

Why delete the copy constructor?

There is no reason why you couldn't have a copy constructor in your class. You can copy the entire state of a late_init if T itself is copyable. Moves should also be possible.

Unnecessary use of explicit

Making a constructor explicit only makes sense if it takes exactly one argument, as then it will prevent it from being an implicit converting constructor. But it's never a converting constructor with either zero or more than one argument.

Alternatives for your use case

You say that the use case for this is for some complex initializions of an object that has no default constructor. There are several ways around that without creating your own class. First, I already mentioned std::optional:

std::optional<my_class> my_object;
{
    example_raii_class temp_resource{ … };
    my_object.emplace(…, temp_resource.something(), …);
}

Or you could use an immediately invoked lambda to wrap complex code and return the value you want, which since C++17 would work even for types that are neither copyable nor movable:

my_class my_object = [&](){
    example_raii_class temp_resource{ … };
    return my_class{…, temp_resource.something(), …};
}();
\$\endgroup\$
2
  • \$\begingroup\$ I totally forgot about std::optional. I’ve been working on a C++14 project for some years now and mentally did away with std::optional as it’s not there, obviously. \$\endgroup\$
    – Bolpat
    Mar 7 at 13:59
  • 1
    \$\begingroup\$ I ended up using the immediately invoked lambda. \$\endgroup\$
    – Bolpat
    Mar 11 at 15:19
3
\$\begingroup\$

This is not meant to be a standalone review, but rather an addendum to @G.Sliepen’s review. Basically, I concur with everything in their review… including that if all you’re interested in is late initialization, std::optional or IILEs are the better way to go.

However, this type could actually be very useful. I am thinking of making something quite like it when I finally get around to updating my personal code library. What you are basically making is your own optional type… and there are very good reasons for why you might want to do that, which I’ll get into later.

So although I know you’ve already decided to use an IILE for your particular problem, I will be reviewing this type as a “better optional”… or something “optional-like”.

Don’t avoid the engaged flag

So the type’s non-static data members are:

    alignas(T) std::byte buffer[sizeof(T)];
#ifndef NDEBUG
    bool initialized = false;
#endif

The idea being, I suppose, that in “release” mode, the size of late_init<T> is the same as the size of T. So all the checks using initialized are done in “debug” mode only.

This is a really, really bad idea.

I sympathize with the chagrin at having to pay for the extra boolean, but the damage that can be done if a single mistake slips through, coupled with the rather high likelihood of such a mistake happening, makes trying to go without the check far, far, far too risky.

You are just asking for a catastrophic failure or—much, much worse—a subtle bug that can be exploited for a security leak. And would be a really nasty bug, too; one that could bite you from both ends. You could end up with serious problems if you mistakenly assume an uninitialized object is initialized, and/or if you mistakenly assume an initialized object is uninitialized.

So you need the flag… but that doesn’t necessarily mean you need to pay for the flag.

Let’s start with a simple function to print the sizes for T, std::optional<T>, and a custom-rolled optional_t<T>:

template <typename T>
auto print_sizes()
{
    std::cout << "  sizeof T:                " << sizeof(T) << '\n';
    std::cout << "  sizeof std::optional<T>: " << sizeof(std::optional<T>) << '\n';
    std::cout << "  sizeof optional_t<T>:    " << sizeof(optional_t<T>) << '\n';
}

Now we make a basic implementation for that custom type:

template <typename T>
struct optional_t
{
    alignas(T) std::byte buffer[sizeof(T) + 1];
};

I’ve added an extra byte to the buffer to store the flag.

With an empty type (struct empty_t {};), we get more or less what you’d expect:

empty_t:
  sizeof T:                1
  sizeof std::optional<T>: 2
  sizeof optional_t<T>:    2

But we don’t actually need the space for the empty type in the optional. It’s empty. (For technical reasons, even empty types have a size of 1; that can’t be avoided. But empty types can be optimized away, for example, by using the empty-base optimization trick, or [[no_unique_address]].) So, in theory, the size of optional<empty_t> could be 1 (because it can’t be zero, and anyway we need at least one byte for the flag). Can we achieve this?

Maybe:

template <typename T>
struct optional_t
{
    alignas(T) std::byte buffer[(std::is_empty_v<T> ? 0 : sizeof(T)) + 1];
};

But let’s consider another angle. To store a value plus a flag generally, we need space for the value and for the flag… but there are some types that have byte patterns that they can never be equal to. For example, class that wraps a pointer that can never be nullptr can never have a nullptr value. That sounds specious and trite, but it’s a powerful observation. It means that we can store just that class in an optional type, and use a check for nullptr instead of a separate flag.

I’m not going to roll out a whole implementation of this idea here, but this is one possible way you might do it:

First, we need an API to detect illegal byte patterns. Maybe something like:

template <typename T>
constexpr auto is_illegal_optional_byte_pattern_for(std::span<std::byte, sizeof(T)>) noexcept -> bool;

Then, for any type that has an illegal byte pattern, we can define that function.

Then we need a type trait that detects whenever a type has that function defined. It could be called has_illegal_optional_byte_pattern<T>.

And with that, the optional type could be something like:

template <typename T>
struct optional_t
{
    alignas(T) std::byte buffer[
        (std::is_empty_v<T> ? 0 : sizeof(T))
        + (not has_illegal_optional_byte_pattern_v<T>)
    ];

    constexpr auto has_value() const noexcept -> bool
    {
        if constexpr (has_illegal_optional_byte_pattern_v<T>)
        {
            return is_illegal_optional_byte_pattern_for<T>(std::span<std::byte, sizeof(T)>(buffer, sizeof(T)));
        }
        else
        {
            return buffer[sizeof(T)] != std::byte{};
        }
    }
};

Which means the size of the optional type is the same as the value type whenever the value type has illegal byte patterns.

This would make your optional type a thousand times better than the one in most standard library implementations.

But this doesn’t need to be a clone of std::optional. Unlike std::optional, maybe once engaged it could stay engaged (and attempts to initialize it more than once could trigger errors). That is less flexible than std::optional, but safer to use, because once you know it’s engaged, it’s engaged until destruction.

Anywho, the bottom line is that you should not remove the flag. Bite the bullet and accept that you need it where you need it, but when possible it can be optimized away without sacrificing safety.

Don’t provide a conversion operator to T&

And especially don’t provide an implicit conversion operator to T&. You mentioned that you consider this a “necessary evil”. I disagree. It’s just evil. Not only is it not necessary, it is actively wrong.

The reason for this is simple: the type isn’t always convertible to T&. Sometimes it doesn’t contain a T.

Your current implementation “deals” with an uninitialized T by either crashing (in “debug” mode) or… UB (in “release” mode). Not great. Worse, this can happen invisibly, in otherwise harmless-looking code.

At a higher level, it’s just a really bad idea in general to have a “T or None” have the same interface as a “T”. Just think about it: you have some code that currently uses T, but then you change it to Maybe<T>… so what happens when the Maybe<T> is uninitialized? Crash and boom is the only option: either throw, terminate, or UB.

The fact that you’ve marked the conversion noexcept really damns it, because noexcept implies the function cannot fail. But it can. And when it does, that means either termination or UB. Because of the noexcept, you don’t even have the option to throw. (Not that throwing would be a particularly great idea here. This is hardly a recoverable error.)

It would be better if you got compile errors when you replaced a T with a Maybe<T>, so you could at least put checks around the code that currently assumes an initialized T. Using the compiler to save you from terrible consequences is the hallmark of a good C++ coder. Greasing your spiral down to UB is not.

If you really don’t want the noise of a .get() or .value() member, you could just do: auto& val = opt.value();, and then use val. It’s one extra line… but more importantly, it’s a line that signals “here be dragons”… “this is a place where something might go wrong, and I am aware of that, but for some reason I ‘know’ it won’t” (possibly because of an earlier check, or some other context). Also, this pattern would even be more efficient than the conversion operator, because you won’t need to keep doing the check.

Don’t use braced construction

In your init():

    template<typename... Args>
    void init(Args&&... args) noexcept(noexcept(T{ std::forward<Args>(args)... }))
    {
        assert(!initialized);
        new (buffer) T{ std::forward<Args>(args)... };
#ifndef NDEBUG
        initialized = true;
#endif
    }

… you do the actual construction using direct-list-initialization… which is not the same same as direct-initialization. (That is if Args is not empty; if it is, then it is value-initialization, in which case there is no difference between parentheses and braces.)

We could really get into the weeds here, but I think the easiest way to illustrate the problem is by asking a simple question: Given auto v = lazy_init<std::vector<int>>{};, what should v.init(3, 4); do? Should it initialize a vector with contents “4, 4, 4”, or with contents “3, 4”?

And a follow-up question: How would I control which option I wanted?

Because you have used list initialization, it is impossible to do direct initialization. .init(3, 4) gives me a vector with “3, 4”. If I want to get 3 copies of the value 4… I can’t. (I mean, other than manually writing that out explicitly, which is silly and wasteful.)

If you had instead done new (buffer) T(std::forward<Args>(args)...); (note the parentheses instead of braces), then .init(3, 4) would give me “4, 4, 4”. And if I wanted “3, 4”, I could write .init({3, 4}). Which actually makes more sense, in my opinion.

But there’s more.

Take a look at your noexcept specification:

noexcept(noexcept(T{ std::forward<Args>(args)... }))

Wouldn’t this be more readable:

noexcept(std::is_nothrow_constructible_v<T, Args...>)

Except you can’t use that so long as you’re using list initialization and not direct initialization.

Also, as of C++20, you could use construct_at() instead of placement new… but again, only for direct initialization. And you might want to do that, because fucking around with naked new is always fraught. For example, did you intend to use class-specific overrides of new? Because if not, you should have written ::new. And are you sure you’re calling placement-new at all? Because you’re actually calling operator new(std::size_t, T*)… not operator new(std::size_t, void*), though it may fall back on that. 🢂MAY🢀.

Basically, list initialization is generally the default choice when you’re actually doing concrete initialization… but when you’re writing generic code, you should always direct initialize. The reason is that if you’ve written a generic function in terms of direct initialization, you get a matching non-initializer list constructor by default but can always opt in to a matching initializer list constructor simply by explicitly using an initializer list… but if you’ve written it in terms of list initialization, there is no way to opt out of the matching initialzer list constructor and select a matching non-initializer list constructor.

You will notice that whenever a standard type has an emplace constructor or function, they always do direct initialization by default. They usually provide an overload that takes an initializer list, too, for ergonomics. (Without it, you’d have to spell out something like std::initializer_list<U>({...}) instead of just {...}.)

So init() should probably look something like:

template <typename... Args>
constexpr auto init(Args&&... args) noexcept(std::is_nothrow_constructible_v<T, Args...>)
{
    assert(not has_value());

    std::ranges::construct_at(reinterpret_cast<T*>(buffer), std::forward<Args>(args)...);

    _set_has_value();
}

And you should probably provide an overload like this:

template <typename U, typename... Args>
constexpr auto init(std::initializer_list<U> il, Args&&... args) noexcept(std::is_nothrow_constructible_v<T, std::initializer_list<U>, Args...>)
{
    assert(not has_value());

    std::ranges::construct_at(reinterpret_cast<T*>(buffer), il, std::forward<Args>(args)...);

    _set_has_value();
}

Be very careful with the special functions

So you’ve deleted the copy constructor, but as @G.Sliepen noted, you don’t need to. However, if you want to allow for copying or moving, you have to be very careful.

The copy/move constructors are a little simpler, because there are only two situations you need to be aware of: whether you are copying/moving from an engaged object or not.

So, for starters, something like this:

constexpr lazy_init(lazy_init const& other) noexcept(/*...*/)
{
    if (other.initialized)
        init(reinterpret_cast<T*>(buffer), other);
}

The noexcept spec can just be is_nothrow_copy_contructible_v<T>. The move constructor is basically the same, too.

So is that everything to say about copy/move construction? Well, not quite. But we’ll get back to that. First let’s look at copy/move assignment.

With copy/move assignment, there are four possible situations you need to be aware of:

RHS engaged RHS not engaged
LHS engaged Normal copy/move assign Destruct
LHS not engaged Copy/move construct Do nothing

Note also that sometimes you won’t actually be doing a copy/move assignment in your copy/move assignment operator… you might be doing a copy/move construction. This might be weird, and it might even make sense, if default construction is no-fail, to actually default construct first then copy/move assign… rather than copy/move construct. But I’ll leave the decision about whether to do that up to the type’s author.

But there’s another factor to consider, too, and that is trivial copyability/movability. There is nothing you can do about trivial default construction, because the nature of the type makes that impossible. However, you can still allow for trivial copy/move. And that might be something worth preserving if T is trivially copyable/movable.

All told, you don’t need to—and shouldn’t—delete the copy/move ops. But they are not simple to implement, and you will have to make some deep decisions, so take care.

explicit is not necessary… but might still be a good idea

It’s true that explicit is primarily to make converting constructors explicit… but that is not its only function.

Try this code:

struct A
{
};

struct B
{
    explicit B() = default;
};

template <typename T>
auto foo() -> T
{
    return {};
}

auto main() -> int
{
    foo<A>();
    //foo<B>();
}

See what happens when you uncomment that second line in main().

Now try this code:

struct A
{
};

struct B
{
    explicit B() = default;
};

auto foo(A)
{
}

auto bar(B)
{
}

auto main() -> int
{
    foo(A{});
    bar(B{});

    foo({});
    //bar({});
}

Again, uncomment and observe.

Finally, try this code:

struct A
{
    A(int, double) {}
};

struct B
{
    explicit B(int, double) {}
};

auto foo(A)
{
}

auto bar(B)
{
}

auto main() -> int
{
    foo(A{1, 2.0});
    bar(B{1, 2.0});

    foo({1, 2.0});
    //bar({1, 2.0});
}

Once again, uncomment, observe.

As you can see, explicit does matter for non-converting constructors. Basically, it makes it required to spell out the type (CTAD notwithstanding, of course).

So… why might you want this? Well, there are certain types that should never be silently deduced. Tag types for example; something like std::in_place_t should never be deduced from {}. It should always be explicitly spelled out either as std::in_place_t{} or, better: std::in_place. And, thus, std::in_place_t has an explicit default constructor.

Does this logic apply to lazy_init? Well, that’s up to the type’s designer. But I would suggest there is at least some merit to not wanting a function call func({}) to silently go from default constructing a T to constructing an uninitialized T (via lazy_init<T>’s default constructor). Forcing someone to spell out func(lazy_init<T>{}) at least makes it obvious what’s going on.

Your mileage may vary, but this is at least something to keep in mind. It’s not an obvious or simple decision. If you look at std::optional’s constructors, you’ll notice that there are some complex rules about which constructors are explicit, and when.

final is pointless and counter-productive

Marking this class final serves no purpose. And it will probably just end up pissing people off.

final might produce some very slightly more optimized code in some specific situations involving virtual function calls. This class has no virtual functions. Thus, final does literally nothing… literally absolutely nothing… for this class.

Except… it prevents deriving from this class. “But that’s not a problem,” you say, “because it never makes sense to derive from this class.”

Are you sure?

Are you really, really, really sure?

Because there are a lot of advanced C++ tricks that rely on deriving from types that no-one thought would be derived from. There is the empty base optimization, and the overload pattern, just to name the first two that popped into my head.

Basically, there is no good argument for marking this class final… which generally true. final should never be used, except in very rare circumstances.

\$\endgroup\$
3
  • \$\begingroup\$ Wow, this was really insightful. I don’t remember when I learned that much in such a short amount of time. There are some ideas that I already thought about. \$\endgroup\$
    – Bolpat
    Mar 19 at 14:01
  • \$\begingroup\$ About final, I did that out of habit. Your remarks are (probably – from my perspective) true for libraries. I never wrote C++ libraries. The C++ code I write is application code. There, to us, final means: “This class is not written with inheritance in mind.” And instead of UB for calling a non-virtual base destructor, we get a compile error. The final annotation is relevant because some classes are written intended to be derived from; they have, e.g., virtual destructors. I guess, practically speaking, final means different things in library and application code. \$\endgroup\$
    – Bolpat
    Mar 19 at 14:01
  • \$\begingroup\$ About the copy/move constructors and assignment, I thought about those and concluded that it’s not really clear what those would mean, and I didn’t need them anyways. In another language, late_init need not even be a type constructor, but a variable annotation, and the language would enforce that it’s initialized before it’s used. The operator T& was in that spirit, but you definitely convinced me, if that were in a library, it would be bad. \$\endgroup\$
    – Bolpat
    Mar 19 at 14:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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