From 6a4f2512b78b4de4003b38f3e67b66c7161ffcee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 8 Feb 2016 12:34:21 +0100 Subject: [PATCH 1/4] narrow: Also check if a value has changed sign after cast. Fixes https://github.com/Microsoft/GSL/issues/222. --- include/gsl_util.h | 42 ++++++++++++++++++++-------------------- tests/utils_tests.cpp | 45 ++++++++++++++++++++++++++++--------------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/include/gsl_util.h b/include/gsl_util.h index e38868c..3769b8a 100644 --- a/include/gsl_util.h +++ b/include/gsl_util.h @@ -1,17 +1,17 @@ -/////////////////////////////////////////////////////////////////////////////// -// -// 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. -// +/////////////////////////////////////////////////////////////////////////////// +// +// 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. +// /////////////////////////////////////////////////////////////////////////////// #pragma once @@ -28,13 +28,13 @@ // No MSVC does constexpr fully yet #pragma push_macro("constexpr") -#define constexpr +#define constexpr // MSVC 2013 workarounds #if _MSC_VER <= 1800 -// noexcept is not understood +// noexcept is not understood #pragma push_macro("noexcept") -#define noexcept +#define noexcept // turn off some misguided warnings #pragma warning(push) @@ -63,7 +63,7 @@ public: final_act(final_act&& other) noexcept : f_(std::move(other.f_)), invoke_(other.invoke_) { other.invoke_ = false; } - + final_act(const final_act&) = delete; final_act& operator=(const final_act&) = delete; @@ -93,12 +93,12 @@ struct narrowing_error : public std::exception {}; // narrow() : a checked version of narrow_cast() that throws if the cast changed the value template inline T narrow(U u) -{ T t = narrow_cast(u); if (static_cast(t) != u) throw narrowing_error(); return t; } +{ T t = narrow_cast(u); if (static_cast(t) != u || (t < T{}) != (u < U{})) throw narrowing_error(); return t; } // // at() - Bounds-checked way of accessing static arrays, std::array, std::vector // -template +template constexpr T& at(T(&arr)[N], size_t index) { Expects(index < N); return arr[index]; } @@ -122,7 +122,7 @@ constexpr typename Cont::value_type& at(Cont& cont, size_t index) #undef noexcept #pragma pop_macro("noexcept") - + #pragma warning(pop) #endif // _MSC_VER <= 1800 diff --git a/tests/utils_tests.cpp b/tests/utils_tests.cpp index 9406e6b..a46d6e4 100644 --- a/tests/utils_tests.cpp +++ b/tests/utils_tests.cpp @@ -1,20 +1,20 @@ -/////////////////////////////////////////////////////////////////////////////// -// -// 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. -// +/////////////////////////////////////////////////////////////////////////////// +// +// 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. +// /////////////////////////////////////////////////////////////////////////////// -#include +#include #include #include @@ -95,8 +95,21 @@ SUITE(utils_tests) char c = narrow(n); CHECK(c == 120); - n = 300; + n = 300; CHECK_THROW(narrow(n), narrowing_error); + + const auto int32_max = std::numeric_limits::max(); + const auto int32_min = std::numeric_limits::min(); + + CHECK(narrow(int32_t(0)) == 0); + CHECK(narrow(int32_t(1)) == 1); + CHECK(narrow(int32_max) == int32_max); + + CHECK_THROW(narrow(int32_t(-1)), narrowing_error); + CHECK_THROW(narrow(int32_min), narrowing_error); + + n = -42; + CHECK_THROW(narrow(n), narrowing_error); } } From 092a8e53e41da8b71996efa32eae40ff35e5a087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 16 Feb 2016 14:29:55 +0100 Subject: [PATCH 2/4] narrow: Check for changed sign only if types have different signess. --- include/gsl_util.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/include/gsl_util.h b/include/gsl_util.h index 3769b8a..4ac6187 100644 --- a/include/gsl_util.h +++ b/include/gsl_util.h @@ -22,6 +22,7 @@ #include "gsl_assert.h" // Ensures/Expects #include #include +#include #include #ifdef _MSC_VER @@ -90,10 +91,24 @@ inline constexpr T narrow_cast(U u) noexcept struct narrowing_error : public std::exception {}; +template +struct is_same_signess +{ + static const bool value = + std::is_signed::value == std::is_signed::value; +}; + // narrow() : a checked version of narrow_cast() that throws if the cast changed the value template inline T narrow(U u) -{ T t = narrow_cast(u); if (static_cast(t) != u || (t < T{}) != (u < U{})) throw narrowing_error(); return t; } +{ + T t = narrow_cast(u); + if (static_cast(t) != u) + throw narrowing_error(); + if (!is_same_signess::value && ((t < T{}) != (u < U{}))) + throw narrowing_error(); + return t; +} // // at() - Bounds-checked way of accessing static arrays, std::array, std::vector From abae0bd998af603d66e929789e054f916cde622b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 19 Feb 2016 19:03:51 +0100 Subject: [PATCH 3/4] Refactor is_same_signedness. --- include/gsl_util.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/gsl_util.h b/include/gsl_util.h index 4ac6187..b144108 100644 --- a/include/gsl_util.h +++ b/include/gsl_util.h @@ -91,12 +91,12 @@ inline constexpr T narrow_cast(U u) noexcept struct narrowing_error : public std::exception {}; -template -struct is_same_signess +namespace details { - static const bool value = - std::is_signed::value == std::is_signed::value; -}; + template + struct is_same_signedness : public std::integral_constant::value == std::is_signed::value> + {}; +} // narrow() : a checked version of narrow_cast() that throws if the cast changed the value template @@ -105,7 +105,7 @@ inline T narrow(U u) T t = narrow_cast(u); if (static_cast(t) != u) throw narrowing_error(); - if (!is_same_signess::value && ((t < T{}) != (u < U{}))) + if (!details::is_same_signedness::value && ((t < T{}) != (u < U{}))) throw narrowing_error(); return t; } From c2924406e22d30046e559c93ccd97090d2018522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Fri, 19 Feb 2016 19:14:39 +0100 Subject: [PATCH 4/4] Disable MSVC warning 4127 (conditional expression is constant) raised for so instances of narrow(). --- include/gsl_util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/gsl_util.h b/include/gsl_util.h index b144108..316f2a1 100644 --- a/include/gsl_util.h +++ b/include/gsl_util.h @@ -40,6 +40,7 @@ // turn off some misguided warnings #pragma warning(push) #pragma warning(disable: 4351) // warns about newly introduced aggregate initializer behavior +#pragma warning(disable: 4127) // conditional expression is constant #endif // _MSC_VER <= 1800