可移动的QScopedPointer，或Qt和std交叉兼容的unique_pointer。

Question

我是一个爱好程序员，所以我从来没有经历过代码审查(在线或离线)。话虽如此，如下所述：

背景

我广泛地使用Qt，但一直希望QScopedPointer是一个可移动的类型。由于Qt不打算实现这一点，所以我开始自己实现它。这是几年前的事了。停下来大笑

目的性

我的目标不是简单地向QScopedPointer添加移动语义：

• 通过尽可能地保持与标准库的兼容性
  • 添加std::hash和std::swap的部分专门化，
  • 声明预期的类型特征，
  • 并在std::unique_pointer中实现所有的公共成员函数。

• 还可以保持与Qt的兼容性，以便qe::UniquePointer充当QScopedPointer的插入替代。
• 在适当的情况下使用C++11并在以后使用(例如，explicit operator bool)。目前，它至少使用C++14进行编译。

Notes

我保留了Qt风格的删除器，以允许QScopedPointer使用现有的删除器。测试中的lambda和宏是我的快速和肮脏的GTest。

目标

• 我想确保基本面是健全的。尽管做了测试，但有经验的眼球还是不错的。
• 我不使用Boost，也没有太多的标准库经验。我错过了任何using声明吗？我对std::hash的部分专门化完成了吗？
• 我对转换函数最不自信(接受qe::UniquePointer<U, Cleanup>)。我是否犯过明显的错误，可能造成边缘的情况？
• 当然，我可能遗漏了一些测试类根本不进行测试的内容。任何有洞察力的人都将不胜感激。

码

uniquepointer.h

#ifndef QE_CORE_UNIQUEPOINTER_H
#define QE_CORE_UNIQUEPOINTER_H

#include <type_traits>

namespace qe {
namespace detail {

template <class T>
constexpr void assertCompleteType() noexcept
{
    using forced_complete_type = char[sizeof(T) ? 1 : -1];
    (void)sizeof(forced_complete_type);
}

template <class T, class U>
void assertConvertible()
{
    using convertible = char[sizeof(std::is_convertible<T, U>()) ? 1 : -1];
    (void)sizeof(convertible);
}

} // namespace detail

template <typename T>
struct DefaultDeleter
{
    static inline void cleanup(T *pointer)
    {
        if (pointer) {
            qe::detail::assertCompleteType<T>();
            delete pointer;
        }
    }
};

template <class T, class Cleanup = DefaultDeleter<T>>
class UniquePointer
{
public:
    using this_type = UniquePointer<T, Cleanup>;
    using element_type = T;
    using pointer = std::add_pointer_t<T>;
    using const_pointer = std::add_const_t<pointer>;
    using reference = std::add_lvalue_reference_t<T>;
    using const_reference = std::add_const_t<reference>;
    using deleter_type = Cleanup;

    inline UniquePointer(pointer p = nullptr) noexcept : d(p) {}
    inline UniquePointer(this_type && other) noexcept : d(other.take()) { }

    template <class U>
    inline UniquePointer(UniquePointer<U, Cleanup> && other) noexcept(std::is_convertible<U, T>())
        : d(other.take())
    {
        qe::detail::assertConvertible<U, T>();
    }

    inline this_type &operator=(this_type &&other)
    {
        if (*this != other)
            reset(other.take());
        return *this;
    }

    template <class U>
    inline this_type &operator=(UniquePointer<U, Cleanup> &&other)
        noexcept(std::is_convertible<U, T>())
    {
        qe::detail::assertConvertible<U, T>();
        if (*this != other)
            reset(other.take());
        return *this;
    }

    UniquePointer(const this_type &) = delete;
    this_type &operator=(const this_type &) = delete;

    inline ~UniquePointer()
    {
        reset(nullptr);
    }

    pointer take() noexcept
    {
        auto oldD = this->d;
        this->d = nullptr;
        return oldD;
    }

    inline void reset(pointer other = nullptr)
    {
        if (this->d == other)
            return;
        auto oldD = this->d;
        if (oldD)
            deleter_type::cleanup(oldD);
        this->d = other;

    }

    inline pointer data() const noexcept            { return this->d; }
    inline pointer get() const noexcept             { return this->d; }
    inline pointer* addressOf() noexcept            { return &(this->d); }
    explicit operator bool() const noexcept         { return !isNull(); }
    inline bool operator!() const noexcept          { return isNull(); }
    inline reference operator*() const noexcept     { return *(this->d); }
    inline pointer operator->() const noexcept      { return this->d; }
    inline bool isNull() const noexcept             { return !(this->d); }
    inline void swap(UniquePointer &other) noexcept { std::swap(this->d, other.d); }

    template <class U>
    void swap(UniquePointer<U, Cleanup> &other)
        noexcept(std::is_convertible<U, T>() && std::is_convertible<T, U>())
    {
        qe::detail::assertConvertible<U, T>();
        qe::detail::assertConvertible<T, U>();
        std::swap(this->d, other.d);
    }

    pointer release() noexcept              { return take(); }

private:
    pointer d;
};

template <class T, class... Args>
inline UniquePointer<T> makeUnique(Args && ...args)
{
    return UniquePointer<T>(new T(std::forward<Args>(args)...));
}

} //namespace qe

template <class T, class Cleanup>
inline bool operator==(const qe::UniquePointer<T, Cleanup> &lhs, const qe::UniquePointer<T, Cleanup> &rhs) noexcept
{
    return lhs.data() == rhs.data();
}

template<class T, class Cleanup>
inline bool operator ==(const qe::UniquePointer<T, Cleanup> &lhs, std::nullptr_t) noexcept
{
    return lhs.isNull();
}

template<class T, class Cleanup>
inline bool operator ==(std::nullptr_t, const qe::UniquePointer<T, Cleanup> &rhs) noexcept
{
    return rhs.isNull();
}

template <class T, class Cleanup>
inline bool operator!=(const qe::UniquePointer<T, Cleanup> &lhs, const qe::UniquePointer<T, Cleanup> &rhs) noexcept
{
    return lhs.data() != rhs.data();
}

template <class T, class Cleanup>
inline bool operator !=(const qe::UniquePointer<T, Cleanup> &lhs, std::nullptr_t) noexcept
{
    return !lhs.data();
}

template <class T, class Cleanup>
inline bool operator !=(std::nullptr_t, const qe::UniquePointer<T, Cleanup> &rhs) noexcept
{
    return !rhs.data();
}

namespace std {
template <class T, class Cleanup>
inline void swap(qe::UniquePointer<T, Cleanup> &lhs, qe::UniquePointer<T, Cleanup> &rhs) noexcept
{
    lhs.swap(rhs);
}

template <class T, class Cleanup>
struct hash<qe::UniquePointer<T, Cleanup>>
{
    using argument_type = qe::UniquePointer<T, Cleanup>;
    using result_type = std::size_t;
    inline result_type operator()(const argument_type & p) const noexcept
    {
        return std::hash<T *>{}(p.data());
    }
};
} //namespace std

#endif //QE_CORE_UNIQUEPOINTER_H

test.h

#ifndef QE_TEST_TEST_H
#define QE_TEST_TEST_H

#include <assert.h>

auto equal_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs == rhs) && (rhs == lhs);
};

auto not_equal_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs != rhs);
};

auto less_than_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs < rhs);
};

auto less_than_or_equal_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs <= rhs);
};

auto greater_than_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs > rhs);
};

auto greater_than_or_equal_check = [](auto && lhs, auto && rhs) -> bool
{
    return (lhs >= rhs);
};

auto implicit_true_check = [](auto && lhs) -> bool
{
    return lhs ? true : false;
};

auto implicit_false_check = [](auto && lhs) -> bool
{
    return lhs ? false : true;
};

#define EXPECT_EQ(x, y) assert(equal_check(x, y))
#define EXPECT_NE(x, y) assert(not_equal_check(x, y))
#define EXPECT_LT(x, y) assert(less_than_check(x, y))
#define EXPECT_LE(x, y) assert(less_than_or_equal_check(x, y))
#define EXPECT_GT(x, y) assert(greater_than_check(x, y))
#define EXPECT_GE(x, y) assert(greater_than_or_equal_check(x, y))
#define EXPECT_TRUE(x)  assert(implicit_true_check(x))
#define EXPECT_FALSE(x) assert(implicit_false_check(x))

#endif // QE_TEST_TEST_H

main.cpp

#include <vector>
#include "uniquepointer.h"
#include "test.h"

using namespace qe;

struct Struct1
{
    explicit Struct1(int aVal)
        : value(aVal)
    {
        instances++;
    }

    Struct1(const Struct1 &) = default;
    Struct1(Struct1 &&) = default;

    ~Struct1()
    {
        instances--;
    }

    Struct1 &operator =(const Struct1 &) = default;
    Struct1 &operator =(Struct1 &&) = default;

    void incr()
    {
        value++;
    }

    void decr()
    {
        value--;
    }

    int value = 0;
    static int instances;
};

int Struct1::instances = 0;

struct Struct2 : public Struct1
{
    explicit Struct2(const int aVal)
        : Struct1(aVal)
    {

    }
};


struct Struct3 : public Struct1
{
    Struct3(const Struct2 &other)
        : Struct1(other.value)
    {
    }
    Struct3(const Struct2 &&other)
        : Struct1(other.value)
    {
    }
    Struct3 operator=(const Struct2 &other)
    {
        Struct3 ret(other);
        return ret;

    }

    Struct3 operator=(Struct2 &&other)
    {
        Struct3 ret(other);
        other.~Struct2();
        return ret;
    }
};

struct unique_pointer_test
{

    static void run()
    {
        empty_pointer_test();
        basic_pointer_test();
        reset_pointer_test();
        compare_pointer_test();
        swap_pointer_test();
        std_container_test();
    }

    static void empty_pointer_test()
    {
        // Create an empty (ie. nullptr) UniquePointer
        UniquePointer<Struct2> xPtr = nullptr;

        EXPECT_FALSE(xPtr);
        EXPECT_EQ(nullptr, xPtr.get());

        // Reset to nullptr (ie. do nothing)
        xPtr.reset();

        EXPECT_FALSE(xPtr);
        EXPECT_EQ(nullptr,  xPtr.get());
    }

    static void basic_pointer_test()
    {
        {
            // Create a UniquePointer
            UniquePointer<Struct2> xPtr(new Struct2(123));

            EXPECT_TRUE(xPtr);
            EXPECT_NE(nullptr, xPtr.get());

            if (xPtr)
            {
                EXPECT_EQ(123,  xPtr->value);
                EXPECT_EQ(1,    xPtr->instances);
                EXPECT_EQ(1,    Struct1::instances);

                // call a function
                xPtr->incr();
                EXPECT_EQ(124,  xPtr->value);
                (*xPtr).incr();
                EXPECT_EQ(125,  (*xPtr).value);
                xPtr->decr();
                xPtr->decr();

                // Copy construct the UniquePointer, transferring ownership
                UniquePointer<Struct2> yPtr(std::move(xPtr));
                xPtr.reset();

                EXPECT_NE(xPtr, yPtr);
                EXPECT_FALSE(xPtr);
                EXPECT_EQ(nullptr,  xPtr.get());
                EXPECT_TRUE(yPtr);
                EXPECT_NE(nullptr,  yPtr.get());
                EXPECT_EQ(123,   yPtr->value);
                EXPECT_EQ(1,     Struct1::instances);

                if (yPtr)
                {
                    UniquePointer<Struct2> zPtr = std::move(yPtr);
                    yPtr.reset();

                    EXPECT_NE(yPtr,  zPtr);
                    EXPECT_FALSE(yPtr);
                    EXPECT_EQ(nullptr, yPtr.get());
                    EXPECT_TRUE(zPtr);
                    EXPECT_NE(nullptr,  zPtr.get());
                    EXPECT_EQ(123,   zPtr->value);
                    EXPECT_EQ(1,     Struct1::instances);
                }

                EXPECT_FALSE(xPtr);
                EXPECT_EQ(nullptr, xPtr.get());
                EXPECT_FALSE(yPtr);
                EXPECT_EQ(nullptr, yPtr.get());
                EXPECT_EQ(0, Struct1::instances);
            }
            else
            {
                assert(false); //"bool cast operator error"
            }

            EXPECT_FALSE(xPtr);
            EXPECT_EQ(nullptr, xPtr.get());
            EXPECT_EQ(0, Struct1::instances);
        }

        EXPECT_EQ(0, Struct1::instances);
    }

    static void reset_pointer_test()
    {
        // Create an empty (ie. nullptr) UniquePointer
        UniquePointer<Struct2> xPtr;

        // Reset it with a new pointer
        xPtr.reset(new Struct2(123));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(123,  xPtr->value);
        EXPECT_EQ(1,    Struct1::instances);
        Struct2* pX  = xPtr.get();

        // Reset it with another new pointer
        xPtr.reset(new Struct2(234));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(234,  xPtr->value);
        EXPECT_EQ(1,    Struct1::instances);
        EXPECT_NE(pX,   xPtr.get());

        // Move-construct a new UniquePointer to the same object, transferring ownership
        UniquePointer<Struct2> yPtr = std::move(xPtr);
        xPtr.reset();

        EXPECT_NE(xPtr, yPtr);
        EXPECT_FALSE( xPtr);
        EXPECT_EQ(nullptr,  xPtr.get());
        EXPECT_TRUE( yPtr);
        EXPECT_NE(nullptr,  yPtr.get());
        EXPECT_EQ(234,   yPtr->value);
        EXPECT_EQ(1,     Struct1::instances);

        // Reset to nullptr
        yPtr.reset();

        EXPECT_EQ(nullptr,  yPtr.get());
        EXPECT_FALSE(xPtr);
        EXPECT_EQ(nullptr,  xPtr.get());
        EXPECT_EQ(0, Struct1::instances);
    }

    static void compare_pointer_test()
    {
        // Create a UniquePointer
        UniquePointer<Struct2> xPtr(new Struct2(123));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(123,xPtr->value);
        EXPECT_EQ(1, Struct1::instances);
        Struct2* pX = xPtr.get();

        // Create another UniquePointer
        UniquePointer<Struct2> yPtr(new Struct2(234));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(123,xPtr->value);
        EXPECT_EQ(2, Struct1::instances);

        EXPECT_TRUE(yPtr);
        EXPECT_NE(nullptr, yPtr.get());
        EXPECT_EQ(234, yPtr->value);
        Struct2* pY = yPtr.get();


        EXPECT_NE(xPtr, yPtr);
        EXPECT_NE(pY->value, pX->value);
    }

    static void swap_pointer_test()
    {
        // Create a UniquePointer
        UniquePointer<Struct2> xPtr(new Struct2(123));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(123,xPtr->value);
        EXPECT_EQ(1, Struct1::instances);

        // Create another UniquePointer
        UniquePointer<Struct2> yPtr(new Struct2(234));

        EXPECT_TRUE(yPtr);
        EXPECT_NE(nullptr, yPtr.get());
        EXPECT_EQ(234, yPtr->value);
        EXPECT_EQ(2, Struct1::instances);

        EXPECT_LT(xPtr->value, yPtr->value);
        xPtr.swap(yPtr);
        EXPECT_GT(xPtr->value, yPtr->value);
        EXPECT_TRUE(xPtr);
        EXPECT_TRUE(yPtr);
    }

    static void std_container_test()
    {
        // Create a shared_ptr
        UniquePointer<Struct2> xPtr(new Struct2(123));

        EXPECT_TRUE(xPtr);
        EXPECT_NE(nullptr, xPtr.get());
        EXPECT_EQ(123, xPtr->value);
        EXPECT_EQ(1, Struct1::instances);
        Struct2* pX = xPtr.get();

        {
            std::vector<UniquePointer<Struct2> > PtrList;

            // Move-it inside a container, transferring ownership
            PtrList.push_back(std::move(xPtr));

            EXPECT_FALSE(xPtr);
            EXPECT_TRUE( PtrList.back());
            EXPECT_EQ(pX,PtrList.back().get());
            EXPECT_EQ(1, Struct1::instances);

        } // Destructor of the vector releases the last pointer thus destroying the object

        EXPECT_EQ(0, Struct1::instances);
    }

};

int main(int argc, char *argv[])
{
    (void)(argc);
    (void)(argv);

    unique_pointer_test::run();

    return 0;
}

编辑:这是魔盒上的代码

编辑2:删除对C++17的需求。用C++14编译。

Answer 1

好吧，一个激进的想法怎么样：

只需使用std::unique_ptr，您所拥有的唯一额外功能就是.addressOf()。是的，如果您使用QScopedPointerPodDeleter和QScopedPointerDeleteLater，您可能需要编写替代程序，但这并不难：

struct FreeDeleter {
    static void operator()(void* p) noexcept { free(p); }
};
struct DeleteLater {
    template <class T>
    static void operator()(T* p) noexcept { p->deleteLater(); }
};

不管怎么说，现在让我们来批评一下你是如何写自己的一本的：

1. static_assert是在编译时不签出时会导致编译时错误的工具。。学会使用和热爱它。
2. 您可以安全地delete或free为空指针.它被定义为什么都不做。
3. 在类定义和离线成员-函数定义-主体内部，您可以使用注入名称 UniquePointer来引用类。不需要提到模板-参数，除非您希望从同一个模板中获得一些无关的类。
4. 用inline装饰类内定义的是绝对没有意义。
5. 我不知道你为什么要把这个表达式放入模板的规范中。您应该将其用于SFINAE或static_assert，但这完全没有意义。我更喜欢第一个: template ()>::type> UniquePointer(UniquePointer及其他)T>：d(other.take()) {}
6. 我建议在移动分配时做最少的工作:简单地交换。
7. 你知道，简单地委托给你已经写好的东西会简化你的模板化的移动赋值操作符: template 自动operator=(UniquePointer&& other) to，除了->解密类型( (*this =UniquePointer(其他)) //额外的父类是关键的{ are (*this=UniquePointer(Other))}；
8. 复印机和复制赋值-操作符已经被隐式删除。
9. 我想知道为什么您不利用dtor中默认的.reset()参数。
10. 当你试图让你的智能指针对它已经负责的事情负责时，会有一些真正令人不安的事情发生。断言这种情况不会发生可能是有意义的，但你真的确定这不应该做什么吗？
11. 在.reset()中，我还想知道删除器是负责处理空指针，还是智能指针，还是两者兼而有之？您似乎非常喜欢在任何地方测试空指针，不管怎样，编译器应该能够很好地去复制代码。
12. 您的模板化.swap()是完全无用的。同样，应该用于SFINAE的表达式在noexcept-specification中，当主体被编译时，您会在以后强制编译时出错，除非U是T。但是，在这种情况下，非模板版本要求优先级。
13. 用不同的方式编写operator==和operator!=，它们将更加全面和有用:模板 constexpr operator==(const UniquePointer& a，const U& b) be，->解密类型(b == a.d) {返回b == a.d；} template constexpr autooperator==( const U& a，const UniquePointer& b) n，除了-> std::enable_if_t() std::is_null_pointer()，解密类型(a == b.d)> {返回一个== b.d；}，该}还包括与原始指针比较、可能的向下转换和遵循相同模式的智能指针。
14. 您不应该将自己的swap()放在::std中。尽管它可能有效，但它是不允许的。要么是依赖于ADL从与您的类型相同的命名空间中获取，要么是依赖于std::swap() (无论如何，这应该足够好)。

Answer 2

恭喜你，祝你一切顺利。

template <class T>
constexpr void assertCompleteType() noexcept
{
    using forced_complete_type = char[sizeof(T) ? 1 : -1];
    (void)sizeof(forced_complete_type);
}

哈?一个不返回值的表达式，以及一个什么都不返回的表达式？

我认为您希望使用static_assert，而不是在模板实例化中造成错误。

static_assert (sizeof(T) > 0);
static_assert (std::is_convertible_v<T,U>);

inline UniquePointer(pointer p = nullptr) noexcept : d(p) {}
inline UniquePointer(this_type && other) noexcept : d(other.take()) { }

在将函数的主体放入类中时，不必使用inline。

使用常用的名称：take应该是release

在复制构造函数和赋值操作符上使用=delete，以防止它们被自动生成。请提供移动赋值操作符。

template <class U>
inline this_type &operator=(UniquePointer<U, Cleanup> &&other)
    noexcept(std::is_convertible<U, T>())
{
    qe::detail::assertConvertible<U, T>();
    if (*this != other)
        reset(other.take());
    return *this;
}

is_convertible在noexcept中是没有意义的。如果你在身体里做的事永远不会扔，那是不扔的。如果这是一个让SFINAE收起的伎俩，那就行不通了。同样，assertConvertible根本不属于身体！如果转换是可能的，您希望这种形式的operator=只匹配重载。

也就是说，使用std::enable_if作为额外的虚拟模板参数。

同时，在没有进行任何转换的情况下将int*分配给long*是没有意义的。您希望检查指针的转换能力，而不是指向的类型！

C++的样式是将*或&与类型放在一起，而不是使用标识符。这是在Stroustrup的第一本书的开头特别提到的，并且是与C风格的一个有意的区别。

    auto oldD = this->d;
    this->d = nullptr;

不要将this->用于正常的成员访问。

在这个函数中，您可以只编写return std::exchange (d, nullptr);。参见CPPreference --您提到需要更好地了解库；这是一个很好的资源。

Answer 3

比较算子

您不需要这么多形式的比较运算符。

首先，!=的单个模板将逆转您所拥有的任何==的含义，所以不要将它们都写两遍。

第二，不要为nullptr编写特殊的表单。我们不鼓励您编写针对nullptr的显式测试，而是使用上下文转换为bool和operator!。实际上，在C++11之前，您不可能有特殊的表单。

如果您有另一个值为null，则将使用通用表单；因此，它必须处理空情况。

但是，您可以提供一个非显式构造函数，将nullptr转换为智能指针类型。