Dev/annagrin/sloppy not null (#712)

* Added c++17 test configurations for clang5.0 and clang6.0

* added transition helper sloppy_not_null

* Moved gsl_transition to a samples folder

* Fixed build break and static analysis warnings

samples/gsl_transition

@@ -0,0 +1,120 @@
+///////////////////////////////////////////////////////////////////////////////
+//
+// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
+//
+// This code is licensed under the MIT License (MIT).
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+///////////////////////////////////////////////////////////////////////////////
+
+#ifndef GSL_TRANSITION_H
+#define GSL_TRANSITION_H
+
+#include <gsl/gsl_assert>  // for Ensures, Expects
+#include <gsl/pointers> // for gsl::not_null
+
+#if defined(_MSC_VER) && _MSC_VER < 1910
+#pragma push_macro("constexpr")
+#define constexpr /*constexpr*/
+
+#endif                          // defined(_MSC_VER) && _MSC_VER < 1910
+
+namespace gsl_helpers
+{
+//
+// sloppy_not_null
+//
+// Restricts a pointer or smart pointer to only hold non-null values,
+// 
+// - provides a sloppy (i.e. no explicit contructor from T) wrapper of gsl::not_null
+// - is temporary, only to be used to incrementally transition of code 
+//   using older version of gsl::not_null to the new one that made the constructor explicit
+// 
+// To make the transition:
+//
+// - replace all occurences of gsl::not_null in your code by sloppy_not_null
+// - compile - compilation should be successful
+// - replace some sloppy_not_nulls by gsl::not_null, fix compilation erros, 
+//   redesign as needed, compile and test
+// - repeat until no sloppy_not_nulls remain
+//
+template <class T>
+class sloppy_not_null: public gsl::not_null<T>
+{
+public:
+
+    template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
+    constexpr sloppy_not_null(U&& u) :
+        gsl::not_null<T>(std::forward<U>(u))
+    {}
+
+    template <typename = std::enable_if_t<!std::is_same<std::nullptr_t, T>::value>>
+    constexpr sloppy_not_null(T u) :
+        gsl::not_null<T>(u)
+    {}
+
+    template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
+    constexpr sloppy_not_null(const gsl::not_null<U>& other) :
+        gsl::not_null<T>(other)
+    {}
+
+    sloppy_not_null(sloppy_not_null&& other) = default;
+    sloppy_not_null(const sloppy_not_null& other) = default;
+    sloppy_not_null& operator=(const sloppy_not_null& other) = default;
+    sloppy_not_null& operator=(const gsl::not_null<T>& other)
+    {
+        gsl::not_null<T>::operator=(other);
+        return *this;
+    }
+
+    // prevents compilation when someone attempts to assign a null pointer constant
+    sloppy_not_null(std::nullptr_t) = delete;
+    sloppy_not_null& operator=(std::nullptr_t) = delete;
+
+    // unwanted operators...pointers only point to single objects!
+    sloppy_not_null& operator++() = delete;
+    sloppy_not_null& operator--() = delete;
+    sloppy_not_null operator++(int) = delete;
+    sloppy_not_null operator--(int) = delete;
+    sloppy_not_null& operator+=(std::ptrdiff_t) = delete;
+    sloppy_not_null& operator-=(std::ptrdiff_t) = delete;
+    void operator[](std::ptrdiff_t) const = delete;
+};
+
+// more unwanted operators
+template <class T, class U>
+std::ptrdiff_t operator-(const sloppy_not_null<T>&, const sloppy_not_null<U>&) = delete;
+template <class T>
+sloppy_not_null<T> operator-(const sloppy_not_null<T>&, std::ptrdiff_t) = delete;
+template <class T>
+sloppy_not_null<T> operator+(const sloppy_not_null<T>&, std::ptrdiff_t) = delete;
+template <class T>
+sloppy_not_null<T> operator+(std::ptrdiff_t, const sloppy_not_null<T>&) = delete;
+
+} // namespace gsl
+
+namespace std
+{
+template <class T>
+struct hash<gsl_helpers::sloppy_not_null<T>>
+{
+    std::size_t operator()(const gsl_helpers::sloppy_not_null<T>& value) const { return hash<T>{}(value); }
+};
+
+} // namespace std
+
+#if defined(_MSC_VER) && _MSC_VER < 1910
+#undef constexpr
+#pragma pop_macro("constexpr")
+
+#endif // defined(_MSC_VER) && _MSC_VER < 1910
+
+#endif // GSL_TRANSITION_H
+

tests/CMakeLists.txt

@@ -107,6 +107,7 @@ add_gsl_test(utils_tests)
 add_gsl_test(owner_tests)
 add_gsl_test(byte_tests)
 add_gsl_test(algorithm_tests)
+add_gsl_test(sloppy_notnull_tests)
 
 
 # No exception tests

tests/no_exception_throw_tests.cpp

@@ -16,6 +16,7 @@
 
 #include <cstdlib>      // for std::exit
 #include <gsl/gsl_util> // for narrow
+#include <gsl/gsl_assert> // for get_terminate
 
 int narrow_no_throw()
 {

tests/notnull_tests.cpp

@@ -25,7 +25,7 @@
 
 #include <catch/catch.hpp> // for AssertionHandler, StringRef, CHECK, TEST_...
 
-#include <gsl/pointers> // for not_null, operator<, operator<=, operator>
+#include <gsl/pointers>       // for not_null, operator<, operator<=, operator>
 
 #include <algorithm> // for addressof
 #include <memory>    // for shared_ptr, make_shared, operator<, opera...
@@ -34,11 +34,10 @@
 #include <string>    // for basic_string, operator==, string, operator<<
 #include <typeinfo>  // for type_info
 
-
-
-namespace gsl {
+namespace gsl
+{
 struct fail_fast;
-}  // namespace gsl
+} // namespace gsl
 
 using namespace gsl;
 
@@ -166,7 +165,7 @@ TEST_CASE("TestNotNullConstructors")
 #ifdef GSL_THROW_ON_CONTRACT_VIOLATION
     int* pi = nullptr;
     CHECK_THROWS_AS(not_null<decltype(pi)>(pi), fail_fast);
-#endif    
+#endif
 }
 
 template<typename T>
@@ -262,7 +261,6 @@ TEST_CASE("TestNotNullRawPointerComparison")
     CHECK((NotNull1(p1) <= NotNull1(p1)) == true);
     CHECK((NotNull1(p1) <= NotNull2(p2)) == (p1 <= p2));
     CHECK((NotNull2(p2) <= NotNull1(p1)) == (p2 <= p1));
-
 }
 
 GSL_SUPPRESS(con.4) // NO-FORMAT: attribute
@@ -272,12 +270,12 @@ TEST_CASE("TestNotNullDereferenceOperator")
         auto sp1 = std::make_shared<NonCopyableNonMovable>();
 
         using NotNullSp1 = not_null<decltype(sp1)>;
-        CHECK(typeid(*sp1) == typeid(*NotNullSp1(sp1))); 
+        CHECK(typeid(*sp1) == typeid(*NotNullSp1(sp1)));
         CHECK(std::addressof(*NotNullSp1(sp1)) == std::addressof(*sp1));
     }
 
     {
-        int ints[1] = { 42 };
+        int ints[1] = {42};
         CustomPtr<int> p1(&ints[0]);
 
         using NotNull1 = not_null<decltype(p1)>;
@@ -360,7 +358,6 @@ TEST_CASE("TestNotNullCustomPtrComparison")
     CHECK((NotNull2(p2) >= NotNull1(p1)) == (p2 >= p1));
 }
 
-
 #if defined(__cplusplus) && (__cplusplus >= 201703L)
 
 GSL_SUPPRESS(con.4) // NO-FORMAT: attribute
@@ -478,3 +475,4 @@ TEST_CASE("TestMakeNotNull")
 }
 
 static_assert(std::is_nothrow_move_constructible<not_null<void *>>::value, "not_null must be no-throw move constructible");
+

tests/sloppy_notnull_tests.cpp

@@ -0,0 +1,124 @@
+///////////////////////////////////////////////////////////////////////////////
+//
+// Copyright (c) 2015 Microsoft Corporation. All rights reserved.
+//
+// This code is licensed under the MIT License (MIT).
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+///////////////////////////////////////////////////////////////////////////////
+
+#ifdef _MSC_VER
+// blanket turn off warnings from CppCoreCheck from catch
+// so people aren't annoyed by them when running the tool.
+#pragma warning(disable : 26440 26426) // from catch
+
+// Fix VS2015 build breaks in Release
+#pragma warning(disable : 4702) // unreachable code
+#endif
+
+#include <catch/catch.hpp>    // for AssertionHandler, StringRef, CHECK, TEST_...
+
+#include <gsl/pointers>           // for not_null, operator<, operator<=, operator>
+#include <samples/gsl_transition> // for sloppy_not_null
+
+namespace gsl
+{
+struct fail_fast;
+} // namespace gsl
+
+using namespace gsl;
+using namespace gsl_helpers;
+
+GSL_SUPPRESS(f.4)  // NO-FORMAT: attribute
+bool helper(not_null<int*> p) { return *p == 12; }
+GSL_SUPPRESS(f.4) // NO-FORMAT: attribute
+bool helper_const(not_null<const int*> p) { return *p == 12; }
+
+GSL_SUPPRESS(f.4) // NO-FORMAT: attribute
+bool sloppy_helper(sloppy_not_null<int*> p) { return *p == 12; }
+GSL_SUPPRESS(f.4) // NO-FORMAT: attribute
+bool sloppy_helper_const(sloppy_not_null<const int*> p) { return *p == 12; }
+
+TEST_CASE("TestSloppyNotNull")
+{
+    {
+        // raw ptr <-> sloppy_not_null
+        int x = 42;
+
+        const sloppy_not_null<int*> snn = &x;
+
+        sloppy_helper(&x);
+        sloppy_helper_const(&x);
+
+        CHECK(*snn == 42);
+    }
+
+    {
+        // sloppy_not_null -> sloppy_not_null
+        int x = 42;
+
+        sloppy_not_null<int*> snn1{&x};
+        const sloppy_not_null<int*> snn2{&x};
+
+        sloppy_helper(snn1);
+        sloppy_helper_const(snn1);
+
+        CHECK(snn1 == snn2);
+    }
+
+    {
+        // sloppy_not_null -> not_null
+        int x = 42;
+
+        sloppy_not_null<int*> snn{&x};
+
+        const not_null<int*> nn1 = snn;
+        const not_null<int*> nn2{snn};
+
+        helper(snn);
+        helper_const(snn);
+
+        CHECK(snn == nn1);
+        CHECK(snn == nn2);
+    }
+
+    {
+        // not_null -> sloppy_not_null
+        int x = 42;
+
+        not_null<int*> nn{&x};
+
+        const sloppy_not_null<int*> snn1{nn};
+        const sloppy_not_null<int*> snn2 = nn;
+
+        sloppy_helper(nn);
+        sloppy_helper_const(nn);
+
+        CHECK(snn1 == nn);
+        CHECK(snn2 == nn);
+
+        std::hash<sloppy_not_null<int*>> hash_snn;
+        std::hash<not_null<int*>> hash_nn;
+
+        CHECK(hash_nn(snn1) == hash_nn(nn));
+        CHECK(hash_snn(snn1) == hash_nn(nn));
+        CHECK(hash_nn(snn1) == hash_nn(snn2));
+        CHECK(hash_snn(snn1) == hash_snn(nn));
+    }
+
+#ifdef CONFIRM_COMPILATION_ERRORS
+    {
+        sloppy_not_null<int*> p{nullptr};
+    }
+#endif
+}
+
+static_assert(std::is_nothrow_move_constructible<sloppy_not_null<void*>>::value,
+              "sloppy_not_null must be no-throw move constructible");