4
\$\begingroup\$

I have written a very simple wrapper around std::shared_ptr that supports lazy initialization. Do you see any problems?

#include <functional>
#include <tuple>
#include <utility>

template <class T, typename ... Args>
class LazySharedPtr {
public:
    LazySharedPtr(Args ... args) :
        ptr(nullptr) {
        this->init = [args = std::make_tuple(std::forward<Args>(args) ...)]() mutable {
            return std::apply([](auto&& ... args) {
                return std::make_shared<T>(std::forward<Args>(args) ...);
                }, std::move(args));
        };
    }

    virtual ~LazySharedPtr() = default;

    bool IsInited() const noexcept {
        return ptr != nullptr;
    }

    void Init() {
        this->InitAndGet();
    }

    std::shared_ptr<T> Get() {
        return (ptr) ? ptr : InitAndGet();
    }

    const std::shared_ptr<T> Get() const {
        return (ptr) ? ptr : InitAndGet();      
    }

    T* operator ->() {
        return this->Get().get();
    }

    const T* operator ->() const {
        return this->Get().get();
    }

    explicit operator bool() const noexcept {
        return this->IsInited();
    }

protected:
    std::function<std::shared_ptr<T>()> init;
    mutable std::shared_ptr<T> ptr;

    std::shared_ptr<T> InitAndGet() const {
        ptr = this->init();
        return ptr;
    }
};

Note: Visual Studio report warning for this->Get().get():

Warning C26815 The pointer is dangling because it points at a temporary instance which was destroyed.

However, I dont see why, because shared_ptr is owned by the class so there should alway be at least one instance "alive". Imho, This warning is not reported by compiler, only by Intellisense.

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

A really useful idea - well worth creating.

Missing #include <memory>. With that fixed, I get almost clean compilation:

255367.cpp:9:5: warning: ‘LazySharedPtr<int, int>::init’ should be initialized in the member initialization list [-Weffc++]

I agree, we should use the initialisation list for init:

LazySharedPtr(Args ... args)
    : init{[args = std::make_tuple(std::forward<Args>(args) ...)]() mutable
           {
               return std::apply([](auto&& ... args) {
                                     return std::make_shared<T>(std::forward<Args>(args) ...);
                                 }, std::move(args));
           }},
      ptr{}
{}

I don't see why we have the inner lambda. There's no good reason not to pass std::make_shared() directly to std::apply(), like this:

LazySharedPtr(Args ... args)
    : init{[args = std::make_tuple(std::forward<Args>(args) ...)]() mutable
           {
               return std::apply(std::make_shared<T, Args...>, std::move(args));
           }},
      ptr{}
{}

Finally here, std::forward() is the wrong function to use on args, which is not a forwarding reference. Use std::move(), since we take the arguments by value.


I think the class could be easier to use. There's no need for the argument types to be part of the class itself, as they are erased by std::function. So make the class template have only T as argument. Otherwise we need separate code paths for our smart pointers if they are created differently. Much better to separate the type that is essential to the LazySharedPtr from those which are accidental:

template <class T>
class LazySharedPtr {
public:
    template <typename ... Args>
    LazySharedPtr(Args ... args);
}

There's some functionality missing that I would expect from a drop-in replacement for std::shared_ptr:

  • operator*
  • get()
  • reset()

I'd also expect a LazySharedPtr to be assignable to std::shared_ptr (invoking the creator function in the process). That could reduce the need for these functions (especially reset(), which might not make sense here).

On the other hand, I don't think that LazySharedPtr is intended as a base class for inheritance, so shouldn't need a virtual destructor. And we don't need IsInited(), given we already have operator bool.

Do have a think about what it means to copy a lazy-pointer object. As it stands, copying a materialized instance yields another shared pointer to the same T object, but copying an unmaterialized instance will result in different T objects in the original and the copy. I wonder if this could make it difficult to use correctly; we might want to make this a move-only type, and require casting to std::shared_ptr in order to copy (thus materialising the object).

For efficiency, InitAndGet() shouldn't copy the shared pointer, but return a reference. I'm not fond of the naming - C++ convention uses snake_case for function names.

There's a lot of unnecessary this-> cluttering the code.


Modified code

#include <cstddef>
#include <functional>
#include <memory>
#include <tuple>
#include <utility>

template <class T>
class LazySharedPtr {
public:
    template <typename... Args>
    LazySharedPtr(Args... args)
        : ptr{},
          init{[args = std::make_tuple(std::move(args)...)]() mutable
               { return std::apply(std::make_shared<T, Args...>, std::move(args)); }}
    {}

    // compiler-defaulted copy/move construct and assign, and destructor

    auto operator->() const
    { return object().get(); }

    auto operator*() const
    { return *object(); }

    auto operator[](std::ptrdiff_t idx)
    { return object()[idx]; }

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

    explicit operator std::shared_ptr<T>() const
    { return object(); }

private:
    mutable std::shared_ptr<T> ptr;
    std::function<std::shared_ptr<T>()> init;

    auto& object() const
    {
        if (!ptr) { ptr = init(); }
        return ptr;
    }
};


int main()
{
    LazySharedPtr<int> a{0};
    auto b = std::shared_ptr<int>{a};
    return *b;
}
\$\endgroup\$
6
  • \$\begingroup\$ Shouldn't we return a reference from operator* instead of a pointer? Also std::shared_ptr::get() is a const function that returns a non-const pointer, so I think we can get away with only a single function for both the dereference and arrow operators: T& operator*() const and T* operator->() const. (Well, I guess ptr is mutable so we could do that anyway...) \$\endgroup\$
    – user673679
    Commented Jan 29, 2021 at 14:04
  • \$\begingroup\$ Yeah, there’s a bit of const confusion here. For a (smart) pointer that’s const, operator-> should return T* const… not T const*… and the const in T* const is specious on a return type, so basically operator->() const should return T*. But I think that’s all moot, because a lazy type should’t support const member functions at all (even with operator bool… what exactly of the semantics of that supposed to be? IsInited was a more logical name). If you want a const pointer, cast it to std::shared_ptr; but LazySharedPtr shouldn’t support being const at all. \$\endgroup\$
    – indi
    Commented Jan 29, 2021 at 17:28
  • 1
    \$\begingroup\$ Also, the std::forward in the constructor is really misleading. [args = std::tuple{std::move(args)...}] is much clearer about what’s going on. (And shorter!) \$\endgroup\$
    – indi
    Commented Jan 29, 2021 at 17:28
  • \$\begingroup\$ Yes, I rushed the operator*() and friends. That's not the meat of the review, though, and I'm sure that's easily corrected. \$\endgroup\$ Commented Jan 30, 2021 at 11:08
  • \$\begingroup\$ If you want a move-only class that creates a shared_ptr when materialized, wouldn’t that be a LazyUniquePtr that can be converted to either a unique_ptr or ahared_ptr, materializing it? \$\endgroup\$
    – Davislor
    Commented Apr 5, 2023 at 19:48

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.