From 6241b3faa6626e6ec735dfd74ae87e92123bf191 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 13 Aug 2018 09:55:48 -0700 Subject: [PATCH] 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 | 120 ++++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/no_exception_throw_tests.cpp | 1 + tests/notnull_tests.cpp | 18 ++--- tests/sloppy_notnull_tests.cpp | 124 +++++++++++++++++++++++++++++ 5 files changed, 254 insertions(+), 10 deletions(-) create mode 100644 samples/gsl_transition create mode 100644 tests/sloppy_notnull_tests.cpp diff --git a/samples/gsl_transition b/samples/gsl_transition new file mode 100644 index 0000000..5e713c4 --- /dev/null +++ b/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 // for Ensures, Expects +#include // 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 sloppy_not_null: public gsl::not_null +{ +public: + + template ::value>> + constexpr sloppy_not_null(U&& u) : + gsl::not_null(std::forward(u)) + {} + + template ::value>> + constexpr sloppy_not_null(T u) : + gsl::not_null(u) + {} + + template ::value>> + constexpr sloppy_not_null(const gsl::not_null& other) : + gsl::not_null(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& other) + { + gsl::not_null::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 +std::ptrdiff_t operator-(const sloppy_not_null&, const sloppy_not_null&) = delete; +template +sloppy_not_null operator-(const sloppy_not_null&, std::ptrdiff_t) = delete; +template +sloppy_not_null operator+(const sloppy_not_null&, std::ptrdiff_t) = delete; +template +sloppy_not_null operator+(std::ptrdiff_t, const sloppy_not_null&) = delete; + +} // namespace gsl + +namespace std +{ +template +struct hash> +{ + std::size_t operator()(const gsl_helpers::sloppy_not_null& value) const { return hash{}(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 + diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5149d2f..2d37a51 100644 --- a/tests/CMakeLists.txt +++ b/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 diff --git a/tests/no_exception_throw_tests.cpp b/tests/no_exception_throw_tests.cpp index 9c491a6..e9f4dac 100644 --- a/tests/no_exception_throw_tests.cpp +++ b/tests/no_exception_throw_tests.cpp @@ -16,6 +16,7 @@ #include // for std::exit #include // for narrow +#include // for get_terminate int narrow_no_throw() { diff --git a/tests/notnull_tests.cpp b/tests/notnull_tests.cpp index 673eab8..668c0a2 100644 --- a/tests/notnull_tests.cpp +++ b/tests/notnull_tests.cpp @@ -25,7 +25,7 @@ #include // for AssertionHandler, StringRef, CHECK, TEST_... -#include // for not_null, operator<, operator<=, operator> +#include // for not_null, operator<, operator<=, operator> #include // for addressof #include // for shared_ptr, make_shared, operator<, opera... @@ -34,11 +34,10 @@ #include // for basic_string, operator==, string, operator<< #include // 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(pi), fail_fast); -#endif +#endif } template @@ -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(); using NotNullSp1 = not_null; - 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 p1(&ints[0]); using NotNull1 = not_null; @@ -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>::value, "not_null must be no-throw move constructible"); + diff --git a/tests/sloppy_notnull_tests.cpp b/tests/sloppy_notnull_tests.cpp new file mode 100644 index 0000000..51983f5 --- /dev/null +++ b/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 // for AssertionHandler, StringRef, CHECK, TEST_... + +#include // for not_null, operator<, operator<=, operator> +#include // 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 p) { return *p == 12; } +GSL_SUPPRESS(f.4) // NO-FORMAT: attribute +bool helper_const(not_null p) { return *p == 12; } + +GSL_SUPPRESS(f.4) // NO-FORMAT: attribute +bool sloppy_helper(sloppy_not_null p) { return *p == 12; } +GSL_SUPPRESS(f.4) // NO-FORMAT: attribute +bool sloppy_helper_const(sloppy_not_null p) { return *p == 12; } + +TEST_CASE("TestSloppyNotNull") +{ + { + // raw ptr <-> sloppy_not_null + int x = 42; + + const sloppy_not_null snn = &x; + + sloppy_helper(&x); + sloppy_helper_const(&x); + + CHECK(*snn == 42); + } + + { + // sloppy_not_null -> sloppy_not_null + int x = 42; + + sloppy_not_null snn1{&x}; + const sloppy_not_null snn2{&x}; + + sloppy_helper(snn1); + sloppy_helper_const(snn1); + + CHECK(snn1 == snn2); + } + + { + // sloppy_not_null -> not_null + int x = 42; + + sloppy_not_null snn{&x}; + + const not_null nn1 = snn; + const not_null nn2{snn}; + + helper(snn); + helper_const(snn); + + CHECK(snn == nn1); + CHECK(snn == nn2); + } + + { + // not_null -> sloppy_not_null + int x = 42; + + not_null nn{&x}; + + const sloppy_not_null snn1{nn}; + const sloppy_not_null snn2 = nn; + + sloppy_helper(nn); + sloppy_helper_const(nn); + + CHECK(snn1 == nn); + CHECK(snn2 == nn); + + std::hash> hash_snn; + std::hash> 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 p{nullptr}; + } +#endif +} + +static_assert(std::is_nothrow_move_constructible>::value, + "sloppy_not_null must be no-throw move constructible");