From 35fb11853ff06d07805b121d928a1ba270532d1a Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Thu, 24 Sep 2015 18:02:37 -0700 Subject: [PATCH 1/5] Fix issues #48 #49 #50 --- include/gsl.h | 15 ++++++++++----- tests/maybenull_tests.cpp | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/include/gsl.h b/include/gsl.h index 1357d76..b78c6e2 100644 --- a/include/gsl.h +++ b/include/gsl.h @@ -162,7 +162,11 @@ private: template class maybe_null_dbg { + template + friend class maybe_null_dbg; public: + static_assert(std::is_constructible::value, "maybe_null's template parameter must be constructible from nullptr"); + maybe_null_dbg() : ptr_(nullptr), tested_(false) {} maybe_null_dbg(const T& p) : ptr_(p), tested_(false) {} @@ -202,8 +206,10 @@ public: bool operator==(const T& rhs) const { tested_ = true; return ptr_ == rhs; } bool operator!=(const T& rhs) const { return !(*this == rhs); } - bool operator==(const maybe_null_dbg& rhs) const { tested_ = true; rhs.tested_ = true; return ptr_ == rhs.ptr_; } - bool operator!=(const maybe_null_dbg& rhs) const { return !(*this == rhs); } + template ::value>> + bool operator==(const maybe_null_dbg& rhs) const { tested_ = true; rhs.tested_ = true; return ptr_ == rhs.ptr_; } + template ::value>> + bool operator!=(const maybe_null_dbg& rhs) const { return !(*this == rhs); } T get() const { fail_fast_assert(tested_); @@ -217,8 +223,6 @@ public: T operator->() const { return get(); } private: - const size_t ptee_size_ = sizeof(*ptr_); // T must be a pointer type - // unwanted operators...pointers only point to single objects! // TODO ensure all arithmetic ops on this type are unavailable maybe_null_dbg& operator++() = delete; @@ -238,6 +242,8 @@ template class maybe_null_ret { public: + static_assert(std::is_constructible::value, "maybe_null's template parameter must be constructible from nullptr"); + maybe_null_ret() : ptr_(nullptr) {} maybe_null_ret(std::nullptr_t) : ptr_(nullptr) {} maybe_null_ret(const T& p) : ptr_(p) {} @@ -280,7 +286,6 @@ private: maybe_null_ret& operator-(size_t) = delete; maybe_null_ret& operator-=(size_t) = delete; - const size_t ptee_size_ = sizeof(*ptr_); // T must be a pointer type T ptr_; }; diff --git a/tests/maybenull_tests.cpp b/tests/maybenull_tests.cpp index 0a9d891..e1244fd 100644 --- a/tests/maybenull_tests.cpp +++ b/tests/maybenull_tests.cpp @@ -241,6 +241,20 @@ SUITE(MaybeNullTests) // Make sure we no longer throw here CHECK(p1.get() != nullptr); } + + TEST(TestMaybeNullPtrT) + { + maybe_null p1; + maybe_null p2; + + CHECK_THROW(p1.get(), fail_fast); + + CHECK(p1 == p2); + + // Make sure we no longer throw here + CHECK(p1.get() == nullptr); + CHECK(p2.get() == nullptr); + } } int main(int, const char *[]) From 1a791992a0bc4b86bedd55768da9585ad9c08e70 Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Fri, 25 Sep 2015 12:16:39 -0700 Subject: [PATCH 2/5] Add equality operators to maybe_nul_ret --- include/gsl.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/gsl.h b/include/gsl.h index b78c6e2..009c777 100644 --- a/include/gsl.h +++ b/include/gsl.h @@ -267,6 +267,9 @@ public: maybe_null_ret& operator=(const T& p) { if (ptr_ != p) { ptr_ = p; } return *this; } maybe_null_ret& operator=(const maybe_null_ret& rhs) = default; + bool operator==(const T& rhs) const { return ptr_ == rhs; } + bool operator!=(const T& rhs) const { return ptr_ != rhs; } + bool present() const { return ptr_ != nullptr; } T get() const { return ptr_; } From a46d6fcf0d69a20f630cd590ba0867a1cf6a8cb4 Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Mon, 28 Sep 2015 15:17:37 -0700 Subject: [PATCH 3/5] Fix issue #49 --- include/gsl.h | 4 +- tests/maybenull_tests.cpp | 80 +++++++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/include/gsl.h b/include/gsl.h index 1405993..a73db6b 100644 --- a/include/gsl.h +++ b/include/gsl.h @@ -223,12 +223,12 @@ public: template ::value>> - maybe_null_dbg(const maybe_null_dbg &other) : ptr_(other.get()), tested_(false) {} + maybe_null_dbg(const maybe_null_dbg &other) : ptr_(other.ptr_), tested_(false) {} template ::value>> maybe_null_dbg& operator=(const maybe_null_dbg &other) { - ptr_ = other.get(); + ptr_ = other.ptr_; tested_ = false; return *this; } diff --git a/tests/maybenull_tests.cpp b/tests/maybenull_tests.cpp index d6b53f0..a889e52 100644 --- a/tests/maybenull_tests.cpp +++ b/tests/maybenull_tests.cpp @@ -256,42 +256,64 @@ SUITE(MaybeNullTests) CHECK(p1.get() != nullptr); } - TEST(TestMaybeNullAssignmentOps) - { - MyBase base; - MyDerived derived; - Unrelated unrelated; + template + void TestMaybeNullAssignmentOpsHelper() + { + MyBase base; + MyDerived derived; + Unrelated unrelated; - not_null nnBase(&base); - not_null nnDerived(&derived); - not_null nnUnrelated(&unrelated); + not_null nnBase(&base); + not_null nnDerived(&derived); + not_null nnUnrelated(&unrelated); - maybe_null_ret mnBase_ret1(&base), mnBase_ret2; - mnBase_ret2 = mnBase_ret1; // maybe_null_ret = maybe_null_ret - mnBase_ret2 = nnBase; // maybe_null_ret = not_null + maybe_null_type::type mnBase_ret1(&base), mnBase_ret2; + mnBase_ret2 = mnBase_ret1; // maybe_null_ret = maybe_null_ret + mnBase_ret2 = nnBase; // maybe_null_ret = not_null - maybe_null_ret mnDerived_ret(&derived); - mnBase_ret2 = mnDerived_ret; // maybe_null_ret = maybe_null_ret - mnBase_ret1 = &derived; // maybe_null_ret = U; - mnBase_ret1 = nnDerived; // maybe_null_ret = not_null + maybe_null_type::type mnDerived_ret(&derived); + mnBase_ret2 = mnDerived_ret; // maybe_null_ret = maybe_null_ret + mnBase_ret1 = &derived; // maybe_null_ret = U; + mnBase_ret1 = nnDerived; // maybe_null_ret = not_null - maybe_null_ret mnUnrelated_ret; - mnUnrelated_ret = &unrelated; // maybe_null_ret = T + maybe_null_type::type mnUnrelated_ret; + mnUnrelated_ret = &unrelated; // maybe_null_ret = T - maybe_null_dbg mnBase_dbg1(&base), mnBase_dbg2; - mnBase_dbg2 = mnBase_dbg1; // maybe_null_dbg = maybe_null_dbg - mnBase_dbg2 = nnBase; // maybe_null_dbg = not_null + maybe_null_type::type mnBase_dbg1(&base), mnBase_dbg2; + mnBase_dbg2 = mnBase_dbg1; // maybe_null_dbg = maybe_null_dbg + mnBase_dbg2 = nnBase; // maybe_null_dbg = not_null - maybe_null_dbg mnDerived_dbg(&derived); - CHECK(mnDerived_dbg.present()); - mnBase_dbg2 = mnDerived_dbg; // maybe_null_dbg = maybe_null_dbg - - mnBase_dbg1 = &derived; // maybe_null_dbg = U; - mnBase_dbg1 = nnDerived; // maybe_null_dbg = not_null + maybe_null_type::type mnDerived_dbg(&derived); + CHECK(mnDerived_dbg.present()); + mnBase_dbg2 = mnDerived_dbg; // maybe_null_dbg = maybe_null_dbg - maybe_null_dbg mnUnrelated_dbg; - mnUnrelated_dbg = &unrelated; // maybe_null_dbg = T - } + mnBase_dbg1 = &derived; // maybe_null_dbg = U; + mnBase_dbg1 = nnDerived; // maybe_null_dbg = not_null + + maybe_null_type::type mnUnrelated_dbg; + mnUnrelated_dbg = &unrelated; // maybe_null_dbg = T + } + + struct maybe_null_ret_type + { + template + using type = maybe_null_ret; + }; + struct maybe_null_dbg_type + { + template + using type = maybe_null_dbg; + }; + + TEST(TestMaybeNullRetAssignmentOps) + { + TestMaybeNullAssignmentOpsHelper(); + } + + TEST(TestMaybeNullDbgAssignmentOps) + { + TestMaybeNullAssignmentOpsHelper(); + } } int main(int, const char *[]) From b29566628e180ce100f9cbdb08d4ba56ab650a01 Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Mon, 28 Sep 2015 18:26:35 -0700 Subject: [PATCH 4/5] Revert "Fix issue #49" This reverts commit a46d6fcf0d69a20f630cd590ba0867a1cf6a8cb4. --- include/gsl.h | 4 +- tests/maybenull_tests.cpp | 80 ++++++++++++++------------------------- 2 files changed, 31 insertions(+), 53 deletions(-) diff --git a/include/gsl.h b/include/gsl.h index a73db6b..1405993 100644 --- a/include/gsl.h +++ b/include/gsl.h @@ -223,12 +223,12 @@ public: template ::value>> - maybe_null_dbg(const maybe_null_dbg &other) : ptr_(other.ptr_), tested_(false) {} + maybe_null_dbg(const maybe_null_dbg &other) : ptr_(other.get()), tested_(false) {} template ::value>> maybe_null_dbg& operator=(const maybe_null_dbg &other) { - ptr_ = other.ptr_; + ptr_ = other.get(); tested_ = false; return *this; } diff --git a/tests/maybenull_tests.cpp b/tests/maybenull_tests.cpp index a889e52..d6b53f0 100644 --- a/tests/maybenull_tests.cpp +++ b/tests/maybenull_tests.cpp @@ -256,64 +256,42 @@ SUITE(MaybeNullTests) CHECK(p1.get() != nullptr); } - template - void TestMaybeNullAssignmentOpsHelper() - { - MyBase base; - MyDerived derived; - Unrelated unrelated; + TEST(TestMaybeNullAssignmentOps) + { + MyBase base; + MyDerived derived; + Unrelated unrelated; - not_null nnBase(&base); - not_null nnDerived(&derived); - not_null nnUnrelated(&unrelated); + not_null nnBase(&base); + not_null nnDerived(&derived); + not_null nnUnrelated(&unrelated); - maybe_null_type::type mnBase_ret1(&base), mnBase_ret2; - mnBase_ret2 = mnBase_ret1; // maybe_null_ret = maybe_null_ret - mnBase_ret2 = nnBase; // maybe_null_ret = not_null + maybe_null_ret mnBase_ret1(&base), mnBase_ret2; + mnBase_ret2 = mnBase_ret1; // maybe_null_ret = maybe_null_ret + mnBase_ret2 = nnBase; // maybe_null_ret = not_null - maybe_null_type::type mnDerived_ret(&derived); - mnBase_ret2 = mnDerived_ret; // maybe_null_ret = maybe_null_ret - mnBase_ret1 = &derived; // maybe_null_ret = U; - mnBase_ret1 = nnDerived; // maybe_null_ret = not_null + maybe_null_ret mnDerived_ret(&derived); + mnBase_ret2 = mnDerived_ret; // maybe_null_ret = maybe_null_ret + mnBase_ret1 = &derived; // maybe_null_ret = U; + mnBase_ret1 = nnDerived; // maybe_null_ret = not_null - maybe_null_type::type mnUnrelated_ret; - mnUnrelated_ret = &unrelated; // maybe_null_ret = T + maybe_null_ret mnUnrelated_ret; + mnUnrelated_ret = &unrelated; // maybe_null_ret = T - maybe_null_type::type mnBase_dbg1(&base), mnBase_dbg2; - mnBase_dbg2 = mnBase_dbg1; // maybe_null_dbg = maybe_null_dbg - mnBase_dbg2 = nnBase; // maybe_null_dbg = not_null + maybe_null_dbg mnBase_dbg1(&base), mnBase_dbg2; + mnBase_dbg2 = mnBase_dbg1; // maybe_null_dbg = maybe_null_dbg + mnBase_dbg2 = nnBase; // maybe_null_dbg = not_null - maybe_null_type::type mnDerived_dbg(&derived); - CHECK(mnDerived_dbg.present()); - mnBase_dbg2 = mnDerived_dbg; // maybe_null_dbg = maybe_null_dbg + maybe_null_dbg mnDerived_dbg(&derived); + CHECK(mnDerived_dbg.present()); + mnBase_dbg2 = mnDerived_dbg; // maybe_null_dbg = maybe_null_dbg + + mnBase_dbg1 = &derived; // maybe_null_dbg = U; + mnBase_dbg1 = nnDerived; // maybe_null_dbg = not_null - mnBase_dbg1 = &derived; // maybe_null_dbg = U; - mnBase_dbg1 = nnDerived; // maybe_null_dbg = not_null - - maybe_null_type::type mnUnrelated_dbg; - mnUnrelated_dbg = &unrelated; // maybe_null_dbg = T - } - - struct maybe_null_ret_type - { - template - using type = maybe_null_ret; - }; - struct maybe_null_dbg_type - { - template - using type = maybe_null_dbg; - }; - - TEST(TestMaybeNullRetAssignmentOps) - { - TestMaybeNullAssignmentOpsHelper(); - } - - TEST(TestMaybeNullDbgAssignmentOps) - { - TestMaybeNullAssignmentOpsHelper(); - } + maybe_null_dbg mnUnrelated_dbg; + mnUnrelated_dbg = &unrelated; // maybe_null_dbg = T + } } int main(int, const char *[]) From 83333419de094f305f1bcaa1e12f7ed801c0cbfa Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Mon, 28 Sep 2015 18:34:04 -0700 Subject: [PATCH 5/5] Add test that reproduces issue --- tests/maybenull_tests.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/maybenull_tests.cpp b/tests/maybenull_tests.cpp index d6b53f0..944307e 100644 --- a/tests/maybenull_tests.cpp +++ b/tests/maybenull_tests.cpp @@ -178,13 +178,19 @@ SUITE(MaybeNullTests) maybe_null q = p; CHECK(q == p); + maybe_null_dbg pdbg = &derived; + CHECK(pdbg.present()); + + maybe_null_dbg qdbg = pdbg; + CHECK(qdbg == pdbg); + #ifdef CONFIRM_COMPILATION_ERRORS maybe_null r = p; maybe_null s = reinterpret_cast(p); #endif maybe_null_dbg t = reinterpret_cast(p.get()); - CHECK_THROW((void)(void*)t.get(), fail_fast); + CHECK_THROW((void)(void*)t.get(), fail_fast); maybe_null_dbg u = reinterpret_cast(p.get()); CHECK(u.present()); CHECK((void*)p.get() == (void*)u.get());