From f1595186ea08dae53acd19f3f66f42420377c91c Mon Sep 17 00:00:00 2001 From: Treb Connell Date: Thu, 8 Oct 2015 11:37:03 -0700 Subject: [PATCH] respond to CR feedback --- include/array_view.h | 38 ++++++------- include/assert.h | 81 --------------------------- include/gsl.h | 2 +- include/{fail_fast.h => gsl_assert.h} | 22 +++++--- 4 files changed, 34 insertions(+), 109 deletions(-) delete mode 100644 include/assert.h rename include/{fail_fast.h => gsl_assert.h} (79%) diff --git a/include/array_view.h b/include/array_view.h index da370e1..bc75e53 100644 --- a/include/array_view.h +++ b/include/array_view.h @@ -29,7 +29,7 @@ #include #include #include -#include "assert.h" +#include "gsl_assert.h" #ifdef _MSC_VER @@ -108,7 +108,7 @@ namespace details constexpr coordinate_facade(std::initializer_list il) { static_assert(std::is_base_of::value, "ConcreteType must be derived from coordinate_facade."); - Expects(il.size() == rank); + Expects(il.size() == rank, "The size of the initializer list must match the rank of the array"); for (size_t i = 0; i < rank; ++i) { elems[i] = begin(il)[i]; @@ -131,13 +131,13 @@ namespace details // Preconditions: component_idx < rank constexpr reference operator[](size_t component_idx) { - Expects(component_idx < rank); + Expects(component_idx < rank, "Component index must be less than rank"); return elems[component_idx]; } // Preconditions: component_idx < rank constexpr const_reference operator[](size_t component_idx) const { - Expects(component_idx < rank); + Expects(component_idx < rank, "Component index must be less than rank"); return elems[component_idx]; } constexpr bool operator==(const ConcreteType& rhs) const noexcept @@ -335,7 +335,7 @@ public: // Preconditions: il.size() == rank constexpr index(std::initializer_list il) { - Expects(il.size() == rank); + Expects(il.size() == rank, "Size of the initializer list must match the rank of the array"); value = begin(il)[0]; } @@ -355,14 +355,14 @@ public: // Preconditions: component_idx < rank constexpr reference operator[](size_type component_idx) noexcept { - Expects(component_idx == 0); + Expects(component_idx == 0, "Component index must be less than rank"); (void)(component_idx); return value; } // Preconditions: component_idx < rank constexpr const_reference operator[](size_type component_idx) const noexcept { - Expects(component_idx == 0); + Expects(component_idx == 0, "Component index must be less than rank"); (void)(component_idx); return value; } @@ -661,7 +661,7 @@ namespace details template SizeType linearize(const T & arr) const { - Expects(arr[Dim] < CurrentRange); + Expects(arr[Dim] < CurrentRange, "Index is out of range"); return static_cast(this->Base::totalSize()) * arr[Dim] + this->Base::template linearize(arr); } @@ -798,8 +798,8 @@ public: constexpr static_bounds(std::initializer_list il) : m_ranges(il.begin()) { - Expects(MyRanges::DynamicNum == il.size()); - Expects(il.size() <= details::SizeTypeTraits::max_value); + Expects(MyRanges::DynamicNum == il.size(), "Size of the initializer list must match the rank of the array"); + Expects(il.size() <= details::SizeTypeTraits::max_value, "Size of the range is larger than the max element of the size type"); } constexpr static_bounds() = default; @@ -1398,8 +1398,8 @@ public: template 1), typename Ret = std::enable_if_t> constexpr Ret operator[](size_type idx) const { - Expects(idx < m_bounds.size()); - Expects(idx * m_bounds.stride() < m_bounds.total_size()); + Expects(idx < m_bounds.size(), "index is out of bounds of the array"); + Expects(idx * m_bounds.stride() < m_bounds.total_size(), "index is out of bounds of the underlying data"); return Ret {m_pdata + idx * m_bounds.stride(), m_bounds.slice()}; } @@ -1946,20 +1946,20 @@ public: template strided_array_view(value_type(&values)[N], bounds_type bounds) : Base(values, std::move(bounds)) { - Expects(this->bounds().total_size() <= N); + Expects(this->bounds().total_size() <= N, "Bounds cross data boundaries"); } // from raw data strided_array_view(pointer ptr, size_type size, bounds_type bounds): Base(ptr, std::move(bounds)) { - Expects(this->bounds().total_size() <= size); + Expects(this->bounds().total_size() <= size, "Bounds cross data boundaries"); } // from array view template > strided_array_view(array_view av, bounds_type bounds) : Base(av.data(), std::move(bounds)) { - Expects(this->bounds().total_size() <= av.bounds().total_size()); + Expects(this->bounds().total_size() <= av.bounds().total_size(), "Bounds cross data boundaries"); } // convertible @@ -2004,7 +2004,7 @@ public: private: static index_type resize_extent(const index_type& extent, size_t d) { - Expects(extent[rank - 1] >= d && (extent[rank-1] % d == 0)); + Expects(extent[rank - 1] >= d && (extent[rank-1] % d == 0), "The last dimension of the array needs to contain a multiple of new type elements"); index_type ret = extent; ret[rank - 1] /= d; @@ -2015,7 +2015,7 @@ private: template > static index_type resize_stride(const index_type& strides, size_t , void * = 0) { - Expects(strides[rank - 1] == 1); + Expects(strides[rank - 1] == 1, "Only strided arrays with regular strides can be resized"); return strides; } @@ -2023,8 +2023,8 @@ private: template 1), typename Dummy = std::enable_if_t> static index_type resize_stride(const index_type& strides, size_t d) { - Expects(strides[rank - 1] == 1); - Expects(strides[rank - 2] >= d && (strides[rank - 2] % d == 0)); + Expects(strides[rank - 1] == 1, "Only strided arrays with regular strides can be resized"); + Expects(strides[rank - 2] >= d && (strides[rank - 2] % d == 0), "The strides must have contiguous chunks of memory that can contain a multiple of new type elements"); for (size_t i = rank - 1; i > 0; --i) fail_fast_assert((strides[i-1] >= strides[i]) && (strides[i-1] % strides[i] == 0), "Only strided arrays with regular strides can be resized"); diff --git a/include/assert.h b/include/assert.h deleted file mode 100644 index 88b8484..0000000 --- a/include/assert.h +++ /dev/null @@ -1,81 +0,0 @@ -/////////////////////////////////////////////////////////////////////////////// -// -// 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 - -#ifndef GSL_UTIL_H -#define GSL_UTIL_H - -#include "fail_fast.h" - -#ifdef _MSC_VER - -// No MSVC does constexpr fully yet -#pragma push_macro("constexpr") -#define constexpr /* nothing */ - -// MSVC 2013 workarounds -#if _MSC_VER <= 1800 - -// noexcept is not understood -#ifndef GSL_THROWS_FOR_TESTING -#define noexcept /* nothing */ -#endif - -// turn off some misguided warnings -#pragma warning(push) -#pragma warning(disable: 4351) // warns about newly introduced aggregate initializer behavior - -#endif // _MSC_VER <= 1800 - -#endif // _MSC_VER - -// In order to test the library, we need it to throw exceptions that we can catch -#ifdef GSL_THROWS_FOR_TESTING -#define noexcept /* nothing */ -#endif // GSL_THROWS_FOR_TESTING - -namespace gsl { - -// -// GSL.assert: assertions -// -#define Expects(x) gsl::fail_fast_assert((x)) -#define Ensures(x) gsl::fail_fast_assert((x)) - -} // namespace gsl - -#ifdef _MSC_VER - -#undef constexpr -#pragma pop_macro("constexpr") - -#if _MSC_VER <= 1800 -#pragma warning(pop) - -#ifndef GSL_THROWS_FOR_TESTING -#pragma undef noexcept -#endif // GSL_THROWS_FOR_TESTING - -#endif // _MSC_VER <= 1800 - -#endif // _MSC_VER - -#if defined(GSL_THROWS_FOR_TESTING) -#undef noexcept -#endif // GSL_THROWS_FOR_TESTING - -#endif // GSL_UTIL_H diff --git a/include/gsl.h b/include/gsl.h index 891297f..f59a1fc 100644 --- a/include/gsl.h +++ b/include/gsl.h @@ -21,7 +21,7 @@ #include "array_view.h" // array_view, strided_array_view... #include "string_view.h" // zstring, string_view, zstring_builder... -#include "assert.h" +#include "gsl_assert.h" #include #ifdef _MSC_VER diff --git a/include/fail_fast.h b/include/gsl_assert.h similarity index 79% rename from include/fail_fast.h rename to include/gsl_assert.h index b9982eb..d87baff 100644 --- a/include/fail_fast.h +++ b/include/gsl_assert.h @@ -16,8 +16,8 @@ #pragma once -#ifndef GSL_FAIL_FAST_H -#define GSL_FAIL_FAST_H +#ifndef GSL_ASSERT_H +#define GSL_ASSERT_H #include @@ -25,8 +25,8 @@ #include #endif -namespace gsl -{ +namespace gsl { + // // Having "fail fast" result in an exception makes unit testing @@ -36,8 +36,8 @@ namespace gsl struct fail_fast : public std::runtime_error { - fail_fast() : std::runtime_error("") {} - explicit fail_fast(char const* const message) : std::runtime_error(message) {} + fail_fast() : std::runtime_error("") {} + explicit fail_fast(char const* const message) : std::runtime_error(message) {} }; inline void fail_fast_assert(bool cond) { if (!cond) throw fail_fast(); } @@ -50,6 +50,12 @@ inline void fail_fast_assert(bool cond, const char* const) { if (!cond) std::ter #endif // GSL_THROWS_FOR_TESTING -} +// +// GSL.assert: assertions +// +#define Expects(x, ...) gsl::fail_fast_assert((x), __VA_ARGS__) +#define Ensures(x, ...) gsl::fail_fast_assert((x), __VA_ARGS__) -#endif // GSL_FAIL_FAST_H +} // namespace gsl + +#endif // GSL_ASSERT_H