From 4487b988e7126a95c491ccfffd4de542a2d9a613 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 14 Aug 2018 22:48:28 +1200 Subject: [PATCH 01/19] Define fp equality function -- Based on checks and defaults used by various testing frameworks (primarily GTest and Catch) -- Moved the header into detail where it should have been to start with (oops) --- include/xlnt/utils/serialisation_utils.hpp | 44 -------- .../header_footer/header_footer_code.cpp | 2 +- source/detail/numeric_utils.hpp | 104 ++++++++++++++++++ source/detail/serialization/xlsx_producer.cpp | 2 +- 4 files changed, 106 insertions(+), 46 deletions(-) delete mode 100644 include/xlnt/utils/serialisation_utils.hpp create mode 100644 source/detail/numeric_utils.hpp diff --git a/include/xlnt/utils/serialisation_utils.hpp b/include/xlnt/utils/serialisation_utils.hpp deleted file mode 100644 index 23bc2ada..00000000 --- a/include/xlnt/utils/serialisation_utils.hpp +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) 2014-2018 Thomas Fussell -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// 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, WRISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE -// -// @license: http://www.opensource.org/licenses/mit-license.php -// @author: see AUTHORS file - -#pragma once - -#include -#include - -namespace xlnt { -/// -/// Takes in any nuber and outputs a string form of that number which will -/// serialise and deserialise without loss of precision -/// -template -std::string serialize_number_to_string(Number num) -{ - // more digits and excel won't match - constexpr int Excel_Digit_Precision = 15; //sf - std::stringstream ss; - ss.precision(Excel_Digit_Precision); - ss << num; - return ss.str(); -} -} diff --git a/source/detail/header_footer/header_footer_code.cpp b/source/detail/header_footer/header_footer_code.cpp index f97ef659..b678ef90 100644 --- a/source/detail/header_footer/header_footer_code.cpp +++ b/source/detail/header_footer/header_footer_code.cpp @@ -22,7 +22,7 @@ // @author: see AUTHORS file #include -#include +#include namespace xlnt { namespace detail { diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp new file mode 100644 index 00000000..1ae25abd --- /dev/null +++ b/source/detail/numeric_utils.hpp @@ -0,0 +1,104 @@ +// Copyright (c) 2014-2018 Thomas Fussell +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// 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, WRISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE +// +// @license: http://www.opensource.org/licenses/mit-license.php +// @author: see AUTHORS file + +#pragma once + +#include +#include +#include + +namespace xlnt { +namespace detail { +/// +/// Takes in any number and outputs a string form of that number which will +/// serialise and deserialise without loss of precision +/// +template +std::string serialize_number_to_string(Number num) +{ + // more digits and excel won't match + constexpr int Excel_Digit_Precision = 15; //sf + std::stringstream ss; + ss.precision(Excel_Digit_Precision); + ss << num; + return ss.str(); +} + +/// +/// constexpr abs +/// +template +constexpr Number abs(Number val) +{ + if (val < Number{0}) + { + return -val; + } + return val; +}; + +/// +/// constexpr max +/// +template +constexpr Number max(Number lval, Number rval) +{ + if (lval < rval) + { + return rval; + } + return lval; +}; + +/// +/// Floating point equality requires a bit of fuzzingdue to the imprecise nature of fp calculation +/// +template +constexpr bool float_equals(const LNumber &lhs, const RNumber &rhs) +{ + static_assert(!std::is_integral::value && !std::is_integral::value, + "Using this function with two integers is just wasting time. Use =="); + + // NANs always compare false with themselves + if ((lhs != lhs) || (rhs != rhs)) + { + return false; + } + // a type that lhs and rhs can agree on + using common_t = std::common_type_t; + // The lower precision epsilon. + // In comparison between different types, the lower precision type must be used for epsilon + constexpr common_t epsilon = detail::max(std::numeric_limits::epsilon(), std::numeric_limits::epsilon()); + // 100 * epsilon selected as an arbitrary range + constexpr common_t fuzz = 100 * epsilon; + // the "epsilon" then needs to be scaled into the comparison range + // epsilon for numeric_limits is valid when abs(x) <1.0, scaling only needs to be upwards + // in particular, this prevents a lhs of 0 from requiring an exact comparison + common_t scaled_fuzz = fuzz * max(xlnt::detail::abs(lhs), common_t{1}); + return ((lhs + scaled_fuzz) >= rhs) && ((rhs + scaled_fuzz) >= lhs); +} + +//static_assert(0.1 != 0.1f, "Built in equality fails"); +//static_assert(float_equals(0.1, 0.1f), "fuzzy equality allows comparison between double and float"); +} // namespace detail +} // namespace xlnt diff --git a/source/detail/serialization/xlsx_producer.cpp b/source/detail/serialization/xlsx_producer.cpp index f8172c16..b4ea1aa8 100644 --- a/source/detail/serialization/xlsx_producer.cpp +++ b/source/detail/serialization/xlsx_producer.cpp @@ -33,12 +33,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include #include From 68589d91ebbae62776bc374dcd2368cfae414f52 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Wed, 15 Aug 2018 18:19:36 +1200 Subject: [PATCH 02/19] default float for epsilon -- Even if both arguments are doubles, they may have been promoted by a previous operation and then the comparison would fail --- source/detail/numeric_utils.hpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 1ae25abd..737992f5 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -73,32 +73,37 @@ constexpr Number max(Number lval, Number rval) /// /// Floating point equality requires a bit of fuzzingdue to the imprecise nature of fp calculation /// -template -constexpr bool float_equals(const LNumber &lhs, const RNumber &rhs) +template // parameter types (deduced) +constexpr bool +float_equals(const LNumber &lhs, const RNumber &rhs, + int epsilon_scale = 100) // scale the "fuzzy" equality. Higher value gives a more tolerant comparison { - static_assert(!std::is_integral::value && !std::is_integral::value, + static_assert(std::is_floating_point::value || std::is_floating_point::value, "Using this function with two integers is just wasting time. Use =="); + static_assert(std::is_floating_point::value, + "Cannot extract epsilon from a number that isn't a floating point type"); // NANs always compare false with themselves - if ((lhs != lhs) || (rhs != rhs)) + if ((lhs != lhs) || (rhs != rhs)) // std::isnan isn't constexpr { return false; } // a type that lhs and rhs can agree on using common_t = std::common_type_t; - // The lower precision epsilon. - // In comparison between different types, the lower precision type must be used for epsilon - constexpr common_t epsilon = detail::max(std::numeric_limits::epsilon(), std::numeric_limits::epsilon()); - // 100 * epsilon selected as an arbitrary range - constexpr common_t fuzz = 100 * epsilon; + // epsilon type defaults to float because even if both args are a higher precision type + // either or both could have been promoted by prior operations + // if a higher precision is required, the template type can be changed + constexpr common_t epsilon = std::numeric_limits::epsilon(); // the "epsilon" then needs to be scaled into the comparison range // epsilon for numeric_limits is valid when abs(x) <1.0, scaling only needs to be upwards // in particular, this prevents a lhs of 0 from requiring an exact comparison - common_t scaled_fuzz = fuzz * max(xlnt::detail::abs(lhs), common_t{1}); + // additionally, a scale factor is applied. + common_t scaled_fuzz = epsilon_scale * epsilon * max(xlnt::detail::abs(lhs), common_t{1}); return ((lhs + scaled_fuzz) >= rhs) && ((rhs + scaled_fuzz) >= lhs); } -//static_assert(0.1 != 0.1f, "Built in equality fails"); -//static_assert(float_equals(0.1, 0.1f), "fuzzy equality allows comparison between double and float"); +static_assert(0.1 != 0.1f, "Built in equality fails when comparing float and double due to imprecision"); +static_assert(float_equals(0.1, 0.1f), "fuzzy equality allows comparison between double and float"); } // namespace detail } // namespace xlnt From 3db87244f12f39f539785c55858d08d47ad55a30 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 16:24:32 +1200 Subject: [PATCH 03/19] Compare numeric cell values using float_equals --- source/worksheet/worksheet.cpp | 49 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/source/worksheet/worksheet.cpp b/source/worksheet/worksheet.cpp index 33386f84..e932556f 100644 --- a/source/worksheet/worksheet.cpp +++ b/source/worksheet/worksheet.cpp @@ -46,6 +46,7 @@ #include #include #include +#include namespace { @@ -98,7 +99,7 @@ void worksheet::create_named_range(const std::string &name, const range_referenc throw invalid_parameter(); //("named range name must be outside the range A1-XFD1048576"); } } - catch (xlnt::invalid_cell_reference&) + catch (xlnt::invalid_cell_reference &) { // name is not a valid reference, that's good } @@ -728,7 +729,8 @@ void worksheet::clear_row(row_t row) { it = d_->cell_map_.erase(it); } - else { + else + { ++it; } } @@ -750,29 +752,28 @@ bool worksheet::compare(const worksheet &other, bool reference) const if (d_->parent_ != other.d_->parent_) return false; - - for (auto &cell : d_->cell_map_) + for (auto &cell : d_->cell_map_) + { + if (other.d_->cell_map_.find(cell.first) == other.d_->cell_map_.end()) { - if (other.d_->cell_map_.find(cell.first) == other.d_->cell_map_.end()) - { - return false; - } - - xlnt::cell this_cell(&cell.second); - xlnt::cell other_cell(&other.d_->cell_map_[cell.first]); - - if (this_cell.data_type() != other_cell.data_type()) - { - return false; - } - - if (this_cell.data_type() == xlnt::cell::type::number - && std::fabs(this_cell.value() - other_cell.value()) > 0.0) - { - return false; - } + return false; } + xlnt::cell this_cell(&cell.second); + xlnt::cell other_cell(&other.d_->cell_map_[cell.first]); + + if (this_cell.data_type() != other_cell.data_type()) + { + return false; + } + + if (this_cell.data_type() == xlnt::cell::type::number + && !detail::float_equals(this_cell.value(), other_cell.value())) + { + return false; + } + } + // todo: missing some comparisons if (d_->auto_filter_ == other.d_->auto_filter_ && d_->views_ == other.d_->views_ @@ -1006,12 +1007,12 @@ bool worksheet::has_phonetic_properties() const return d_->phonetic_properties_.is_set(); } -const phonetic_pr& worksheet::phonetic_properties() const +const phonetic_pr &worksheet::phonetic_properties() const { return d_->phonetic_properties_.get(); } -void worksheet::phonetic_properties(const phonetic_pr& phonetic_props) +void worksheet::phonetic_properties(const phonetic_pr &phonetic_props) { d_->phonetic_properties_.set(phonetic_props); } From 19aad525006f5cd57e8bb598d475e68eb0ab10f4 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 16:31:56 +1200 Subject: [PATCH 04/19] fuzzy floating point comparison with optional --- include/xlnt/utils/optional.hpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp index 8677f24b..4f56c812 100644 --- a/include/xlnt/utils/optional.hpp +++ b/include/xlnt/utils/optional.hpp @@ -25,6 +25,7 @@ #include #include +#include #include namespace xlnt { @@ -49,6 +50,23 @@ class optional using set_move_noexcept_t = typename std::conditional{} && std::is_nothrow_move_assignable{}, std::true_type, std::false_type>::type; using clear_noexcept_t = typename std::conditional{}, std::true_type, std::false_type>::type; #endif + /// + /// Default equality operation, just uses operator== + /// + template >::type * = nullptr> + constexpr bool compare_equal(const T &lhs, const T &rhs) const + { + return lhs == rhs; + } + + /// + /// equality operation for floating point numbers. Provides "fuzzy" equality + /// + template >::type * = nullptr> + constexpr bool compare_equal(const T &lhs, const T &rhs) const + { + return detail::float_equals(lhs, rhs); + } public: /// @@ -281,7 +299,8 @@ public: { return true; } - return value_ref() == other.value_ref(); + // equality is overloaded to provide fuzzy equality when T is a fp number + return compare_equal(value_ref(), other.value_ref()); } /// From 7b05beae90df52de8f12afd467907604562f5003 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 16:34:31 +1200 Subject: [PATCH 05/19] unsupress fp equality warnings --- source/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 186e57fc..b50ba212 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -43,7 +43,6 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-documentation-unknown-command") # ignore unknown commands in Javadoc-style comments set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas") # ignore Windows and GCC pragmas set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-warning-option") # ignore Windows and GCC pragmas - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-float-equal") # don't warn on uses of == for fp types set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-newline-eof") # no longer an issue with post-c++11 standards which mandate include add a newline if neccesary set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-covered-switch-default") # default is often added to switches for completeness or to cover future alternatives set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-exit-time-destructors") # this is just a warning to notify that the destructor will run during exit From 6aa10131a264bafe6e695ea33d5f45d84303673d Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 16:51:53 +1200 Subject: [PATCH 06/19] Fixing compile errors --- include/xlnt/utils/optional.hpp | 8 ++++---- source/detail/numeric_utils.hpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp index 4f56c812..757c90ea 100644 --- a/include/xlnt/utils/optional.hpp +++ b/include/xlnt/utils/optional.hpp @@ -53,8 +53,8 @@ class optional /// /// Default equality operation, just uses operator== /// - template >::type * = nullptr> - constexpr bool compare_equal(const T &lhs, const T &rhs) const + template ::value>::type * = nullptr> + constexpr bool compare_equal(const U &lhs, const U &rhs) const { return lhs == rhs; } @@ -62,8 +62,8 @@ class optional /// /// equality operation for floating point numbers. Provides "fuzzy" equality /// - template >::type * = nullptr> - constexpr bool compare_equal(const T &lhs, const T &rhs) const + template ::value>::type * = nullptr> + constexpr bool compare_equal(const U &lhs, const U &rhs) const { return detail::float_equals(lhs, rhs); } diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 737992f5..2e3a0de9 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -90,7 +90,7 @@ float_equals(const LNumber &lhs, const RNumber &rhs, return false; } // a type that lhs and rhs can agree on - using common_t = std::common_type_t; + using common_t = std::common_type::type; // epsilon type defaults to float because even if both args are a higher precision type // either or both could have been promoted by prior operations // if a higher precision is required, the template type can be changed From edafcd1bb597b69623166a3e411b86baea918355 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 17:23:04 +1200 Subject: [PATCH 07/19] More compile fixes --- source/detail/numeric_utils.hpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 2e3a0de9..990d28b8 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -63,21 +63,32 @@ constexpr Number abs(Number val) template constexpr Number max(Number lval, Number rval) { - if (lval < rval) - { - return rval; - } - return lval; -}; + return (lval < rval) ? rval : lval; +} /// -/// Floating point equality requires a bit of fuzzingdue to the imprecise nature of fp calculation +/// constexpr min +/// +template +constexpr Number min(Number lval, Number rval) +{ + return (lval < rval) ? lval : rval; +} + +/// +/// Floating point equality requires a bit of fuzzing due to the imprecise nature of fp calculation +/// References: +/// - Several blogs/articles were referenced with the following being the most useful +/// -- https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ +/// -- http://realtimecollisiondetection.net/blog/?p=89 +/// - Testing Frameworks {Catch2, Boost, Google}, primarily for selecting the default scale factor +/// -- None of these even remotely agree /// template // parameter types (deduced) -constexpr bool +bool float_equals(const LNumber &lhs, const RNumber &rhs, - int epsilon_scale = 100) // scale the "fuzzy" equality. Higher value gives a more tolerant comparison + int epsilon_scale = 20) // scale the "fuzzy" equality. Higher value gives a more tolerant comparison { static_assert(std::is_floating_point::value || std::is_floating_point::value, "Using this function with two integers is just wasting time. Use =="); @@ -90,7 +101,7 @@ float_equals(const LNumber &lhs, const RNumber &rhs, return false; } // a type that lhs and rhs can agree on - using common_t = std::common_type::type; + using common_t = typename std::common_type::type; // epsilon type defaults to float because even if both args are a higher precision type // either or both could have been promoted by prior operations // if a higher precision is required, the template type can be changed @@ -103,7 +114,5 @@ float_equals(const LNumber &lhs, const RNumber &rhs, return ((lhs + scaled_fuzz) >= rhs) && ((rhs + scaled_fuzz) >= lhs); } -static_assert(0.1 != 0.1f, "Built in equality fails when comparing float and double due to imprecision"); -static_assert(float_equals(0.1, 0.1f), "fuzzy equality allows comparison between double and float"); } // namespace detail } // namespace xlnt From 3a10e661b83fe11aad001b280a21982ec70717ff Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 17:24:34 +1200 Subject: [PATCH 08/19] constexpr concern no longer valid --- source/detail/numeric_utils.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 990d28b8..21c48397 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -96,7 +96,7 @@ float_equals(const LNumber &lhs, const RNumber &rhs, "Cannot extract epsilon from a number that isn't a floating point type"); // NANs always compare false with themselves - if ((lhs != lhs) || (rhs != rhs)) // std::isnan isn't constexpr + if (std::isnan(lhs) || std::isnan(rhs)) { return false; } From d7603964e9f535501e40f6cbd03a76c0d6b96ec2 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 17:28:31 +1200 Subject: [PATCH 09/19] another compile error --- source/detail/numeric_utils.hpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 21c48397..6fba408b 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -50,11 +50,7 @@ std::string serialize_number_to_string(Number num) template constexpr Number abs(Number val) { - if (val < Number{0}) - { - return -val; - } - return val; + return (val < Number{0}) ? -val : val; }; /// From 8b3031951492e155629c6ad96b71463a29647230 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 17:31:25 +1200 Subject: [PATCH 10/19] missing include --- source/detail/numeric_utils.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 6fba408b..0b6458e6 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -24,6 +24,7 @@ #pragma once #include +#include #include #include From b2a514fdbfb1ecf88f9d990bc3d476ec2f68f10f Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 17:55:32 +1200 Subject: [PATCH 11/19] Include without relying on include directories --- include/xlnt/utils/optional.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp index 757c90ea..4f5b6b78 100644 --- a/include/xlnt/utils/optional.hpp +++ b/include/xlnt/utils/optional.hpp @@ -23,9 +23,9 @@ #pragma once -#include -#include -#include +#include "xlnt/xlnt_config.hpp" +#include "xlnt/utils/exceptions.hpp" +#include "../source/detail/numeric_utils.hpp" #include namespace xlnt { From 61e46c934a101f4199073aa487e97d5644235d09 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 18:12:46 +1200 Subject: [PATCH 12/19] resolve float-equals warnings --- include/xlnt/worksheet/sheet_format_properties.hpp | 3 ++- source/detail/implementations/cell_impl.hpp | 3 ++- source/detail/numeric_utils.hpp | 2 +- source/worksheet/page_margins.cpp | 11 ++++++----- source/worksheet/page_setup.cpp | 3 ++- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/include/xlnt/worksheet/sheet_format_properties.hpp b/include/xlnt/worksheet/sheet_format_properties.hpp index 52e4ceb1..dd63685d 100644 --- a/include/xlnt/worksheet/sheet_format_properties.hpp +++ b/include/xlnt/worksheet/sheet_format_properties.hpp @@ -25,6 +25,7 @@ #include #include +#include "../source/detail/numeric_utils.hpp" namespace xlnt { @@ -59,7 +60,7 @@ inline bool operator==(const sheet_format_properties &lhs, const sheet_format_pr { return lhs.base_col_width == rhs.base_col_width && lhs.default_column_width == rhs.default_column_width - && lhs.default_row_height == rhs.default_row_height + && detail::float_equals(lhs.default_row_height, rhs.default_row_height) && lhs.dy_descent == rhs.dy_descent; } diff --git a/source/detail/implementations/cell_impl.hpp b/source/detail/implementations/cell_impl.hpp index 64756770..59916331 100644 --- a/source/detail/implementations/cell_impl.hpp +++ b/source/detail/implementations/cell_impl.hpp @@ -33,6 +33,7 @@ #include #include #include +#include "../numeric_utils.hpp" namespace xlnt { namespace detail { @@ -73,7 +74,7 @@ inline bool operator==(const cell_impl &lhs, const cell_impl &rhs) && lhs.row_ == rhs.row_ && lhs.is_merged_ == rhs.is_merged_ && lhs.value_text_ == rhs.value_text_ - && lhs.value_numeric_ == rhs.value_numeric_ + && float_equals(lhs.value_numeric_, rhs.value_numeric_) && lhs.formula_ == rhs.formula_ && lhs.hyperlink_ == rhs.hyperlink_ && (lhs.format_.is_set() == rhs.format_.is_set() && (!lhs.format_.is_set() || *lhs.format_.get() == *rhs.format_.get())) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 0b6458e6..554c3b13 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -102,7 +102,7 @@ float_equals(const LNumber &lhs, const RNumber &rhs, // epsilon type defaults to float because even if both args are a higher precision type // either or both could have been promoted by prior operations // if a higher precision is required, the template type can be changed - constexpr common_t epsilon = std::numeric_limits::epsilon(); + constexpr common_t epsilon = static_cast(std::numeric_limits::epsilon()); // the "epsilon" then needs to be scaled into the comparison range // epsilon for numeric_limits is valid when abs(x) <1.0, scaling only needs to be upwards // in particular, this prevents a lhs of 0 from requiring an exact comparison diff --git a/source/worksheet/page_margins.cpp b/source/worksheet/page_margins.cpp index 77faec5b..fc1c69c2 100644 --- a/source/worksheet/page_margins.cpp +++ b/source/worksheet/page_margins.cpp @@ -22,6 +22,7 @@ // @license: http://www.opensource.org/licenses/mit-license.php // @author: see AUTHORS file #include +#include "detail/numeric_utils.hpp" namespace xlnt { @@ -91,11 +92,11 @@ void page_margins::footer(double footer) bool page_margins::operator==(const page_margins &rhs) const { - return top_ == rhs.top_ - && left_ == rhs.left_ - && right_ == rhs.right_ - && header_ == rhs.header_ - && footer_ == rhs.footer_; + return detail::float_equals(top_, rhs.top_) + && detail::float_equals(left_,rhs.left_) + && detail::float_equals(right_, rhs.right_) + && detail::float_equals(header_, rhs.header_) + && detail::float_equals(footer_, rhs.footer_); } } // namespace xlnt diff --git a/source/worksheet/page_setup.cpp b/source/worksheet/page_setup.cpp index c832d5f4..087d980c 100644 --- a/source/worksheet/page_setup.cpp +++ b/source/worksheet/page_setup.cpp @@ -22,6 +22,7 @@ // @license: http://www.opensource.org/licenses/mit-license.php // @author: see AUTHORS file #include +#include "detail/numeric_utils.hpp" namespace xlnt { @@ -114,7 +115,7 @@ bool page_setup::operator==(const page_setup &rhs) const && fit_to_page_ == rhs.fit_to_page_ && fit_to_height_ == rhs.fit_to_height_ && fit_to_width_ == rhs.fit_to_width_ - && scale_ == rhs.scale_; + && detail::float_equals(scale_, rhs.scale_); } } // namespace xlnt From 9e78e55c62975685b9f7b60079b0773c17de12f8 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 18:16:39 +1200 Subject: [PATCH 13/19] Another missing include --- source/detail/numeric_utils.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 554c3b13..a50ea802 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -25,6 +25,7 @@ #include #include +#include #include #include From a28f3fb7a9b19f2203fd89b8f2ed5074302bc4de Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 18 Aug 2018 18:37:19 +1200 Subject: [PATCH 14/19] zstream implicit conversion warning --- source/detail/serialization/zstream.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/source/detail/serialization/zstream.cpp b/source/detail/serialization/zstream.cpp index be72bd50..e184d5bb 100644 --- a/source/detail/serialization/zstream.cpp +++ b/source/detail/serialization/zstream.cpp @@ -507,12 +507,12 @@ bool izstream::read_central_header() // Find the header // NOTE: this assumes the zip file header is the last thing written to file... source_stream_.seekg(0, std::ios_base::end); - auto end_position = static_cast(source_stream_.tellg()); + auto end_position = source_stream_.tellg(); auto max_comment_size = std::uint32_t(0xffff); // max size of header auto read_size_before_comment = std::uint32_t(22); - std::uint32_t read_start = max_comment_size + read_size_before_comment; + std::streamoff read_start = max_comment_size + read_size_before_comment; if (read_start > end_position) { @@ -536,11 +536,14 @@ bool izstream::read_central_header() } auto found_header = false; - std::size_t header_index = 0; + std::streamoff header_index = 0; - for (std::size_t i = 0; i < static_cast(read_start - 3); ++i) + for (std::streamoff i = 0; i < read_start - 3; ++i) { - if (buf[i] == 0x50 && buf[i + 1] == 0x4b && buf[i + 2] == 0x05 && buf[i + 3] == 0x06) + if (buf[static_cast(i)] == 0x50 + && buf[static_cast(i) + 1] == 0x4b + && buf[static_cast(i) + 2] == 0x05 + && buf[static_cast(i) + 3] == 0x06) { found_header = true; header_index = i; @@ -554,7 +557,7 @@ bool izstream::read_central_header() } // seek to end of central header and read - source_stream_.seekg(end_position - (read_start - static_cast(header_index))); + source_stream_.seekg(end_position - (read_start - header_index)); /*auto word = */ read_int(source_stream_); auto disk_number1 = read_int(source_stream_); From 95653779638b4664df676d3be6a56dced091a933 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sun, 19 Aug 2018 14:41:00 +1200 Subject: [PATCH 15/19] Adding tests for xlnt::detail::float_equals -- Plenty of comments which should be useful if it becomes useful to tweak the comparison --- tests/CMakeLists.txt | 3 + tests/detail/numeric_util_test_suite.cpp | 112 +++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 tests/detail/numeric_util_test_suite.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 74c24352..7c6ee18e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -15,6 +15,7 @@ if(STATIC_CRT) endif() file(GLOB CELL_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/cell/*.cpp) +file(GLOB DETAIL_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/detail/*.cpp) file(GLOB PACKAGING_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/packaging/*.cpp) file(GLOB STYLES_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/styles/*.cpp) file(GLOB UTILS_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/utils/*.cpp) @@ -23,6 +24,7 @@ file(GLOB WORKSHEET_TESTS ${CMAKE_CURRENT_SOURCE_DIR}/worksheet/*.cpp) set(TESTS ${CELL_TESTS} + ${DETAIL_TESTS} ${PACKAGING_TESTS} ${STYLES_TESTS} ${UTILS_TESTS} @@ -59,6 +61,7 @@ endif() source_group(helpers FILES ${HELPERS}) source_group(runner FILES ${RUNNER}) source_group(tests\\cell FILES ${CELL_TESTS}) +source_group(tests\\detail FILES ${DETAIL_TESTS}) source_group(tests\\packaging FILES ${PACKAGING_TESTS}) source_group(tests\\serialization FILES ${SERIALIZATION_TESTS}) source_group(tests\\styles FILES ${STYLES_TESTS}) diff --git a/tests/detail/numeric_util_test_suite.cpp b/tests/detail/numeric_util_test_suite.cpp new file mode 100644 index 00000000..2e9489fb --- /dev/null +++ b/tests/detail/numeric_util_test_suite.cpp @@ -0,0 +1,112 @@ +// Copyright (c) 2014-2018 Thomas Fussell +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// 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, WRISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE +// +// @license: http://www.opensource.org/licenses/mit-license.php +// @author: see AUTHORS file + +#include "../../source/detail/numeric_utils.hpp" +#include + +class numeric_test_suite : public test_suite +{ +public: + numeric_test_suite() + { + register_test(test_float_equals_zero); + register_test(test_float_equals_large); + } + + void test_float_equals_zero() + { + // comparing relatively small numbers (2.3e-6) with 0 will be true by default + const float comp_val = 2.3e-6; // about the largest difference allowed by default + xlnt_assert(0.f != comp_val); // fail because not exactly equal + xlnt_assert(xlnt::detail::float_equals(0.0, comp_val)); + xlnt_assert(xlnt::detail::float_equals(0.0, -comp_val)); + // fail because diff is out of bounds for fuzzy equality + xlnt_assert(!xlnt::detail::float_equals(0.0, comp_val + 0.1e-6)); + xlnt_assert(!xlnt::detail::float_equals(0.0, -(comp_val + 0.1e-6))); + // if the bounds of comparison are too loose, there are two tweakable knobs to tighten the comparison up + //========================================================== + // #1: reduce the epsilon_scale (default is 20) + // This can bring the range down to FLT_EPSILON (scale factor of 1) + xlnt_assert(!xlnt::detail::float_equals(0.0, comp_val, 10)); + const float closer_comp_val = 1.1e-6; + xlnt_assert(xlnt::detail::float_equals(0.0, closer_comp_val, 10)); + xlnt_assert(!xlnt::detail::float_equals(0.0, closer_comp_val + 0.1e-6, 10)); + xlnt_assert(xlnt::detail::float_equals(0.0, -closer_comp_val, 10)); + xlnt_assert(!xlnt::detail::float_equals(0.0, -(closer_comp_val + 0.1e-6), 10)); + //========================================================== + // #2: specify the epsilon source as a higher precision type (e.g. double) + // This makes the epsilon range quite significantly less + xlnt_assert(!xlnt::detail::float_equals(0.0, comp_val)); + xlnt_assert(!xlnt::detail::float_equals(0.0, closer_comp_val)); + const float tiny_comp_val = 4.4e-15; + xlnt_assert(xlnt::detail::float_equals(0.0, tiny_comp_val)); + xlnt_assert(!xlnt::detail::float_equals(0.0, tiny_comp_val + 0.1e-15)); + xlnt_assert(xlnt::detail::float_equals(0.0, -tiny_comp_val)); + xlnt_assert(!xlnt::detail::float_equals(0.0, -(tiny_comp_val + 0.1e-15))); + //========================================================== + // #3: combine #1 & #2 + // for the tightest default precision, double with a scale of 1 + xlnt_assert(!xlnt::detail::float_equals(0.0, comp_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(0.0, closer_comp_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(0.0, tiny_comp_val, 1)); + const float really_tiny_comp_val = 2.2e-16; // the limit is +/- std::numeric_limits::epsilon() + xlnt_assert(xlnt::detail::float_equals(0.0, really_tiny_comp_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(0.0, really_tiny_comp_val + 0.1e-16, 1)); + xlnt_assert(xlnt::detail::float_equals(0.0, -really_tiny_comp_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(0.0, -(really_tiny_comp_val + 0.1e-16), 1)); + //========================================================== + // in the world of floats, 2.2e-16 is still significantly different to 0.f (smallest representable float is around 1e-38) + // if comparisons are known to involve extremely small numbers (such that +/- 2.2e-16 is too large a band), + // a type that specialises std::numeric_limits::epsilon may be passed as the first template parameter + // the type itself doesn't actually need to have any behaviour as it is only used as the source for epsilon + // struct super_precise{}; + // namespace std { + // template<> numeric_limits { + // double epsilon() { + // return 1e-30; + // } + // } + // } + // float_equals(0.0, 2e-30, 1); // returns true + // float_equals(0.0, 2e-30, 1); // returns false + } + + void test_float_equals_large() + { + const float compare_to = 20e6; + // fp math with arguments of different magnitudes is wierd + xlnt_assert(compare_to == compare_to + 1); // x == x + 1 ... + xlnt_assert(compare_to != compare_to + 10); // x != x + 10 + xlnt_assert(compare_to != compare_to - 10); // x != x - 10 + // if the same epsilon was used for comparison of large values as the values around one + // we'd have all the issues around zero again + xlnt_assert(xlnt::detail::float_equals(compare_to, compare_to + 49)); + xlnt_assert(!xlnt::detail::float_equals(compare_to, compare_to + 50)); + xlnt_assert(xlnt::detail::float_equals(compare_to, compare_to - 49)); + xlnt_assert(!xlnt::detail::float_equals(compare_to, compare_to - 50)); + // float_equals also scales epsilon up to match the magnitude of its arguments + // all the same options are available for increasing/decreasing the precision of the comparison + // however the the epsilon source should always be of equal or lesser precision than the arguments when away from zero + } +}; +static numeric_test_suite x; \ No newline at end of file From 4188d35cafca3f1540276514f2d8d286029adf8d Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sun, 19 Aug 2018 15:11:57 +1200 Subject: [PATCH 16/19] Add tests for the other small utilities --- source/detail/numeric_utils.hpp | 8 +-- tests/detail/numeric_util_test_suite.cpp | 67 ++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index a50ea802..30f49f89 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -58,8 +58,8 @@ constexpr Number abs(Number val) /// /// constexpr max /// -template -constexpr Number max(Number lval, Number rval) +template +constexpr typename std::common_type::type max(NumberL lval, NumberR rval) { return (lval < rval) ? rval : lval; } @@ -67,8 +67,8 @@ constexpr Number max(Number lval, Number rval) /// /// constexpr min /// -template -constexpr Number min(Number lval, Number rval) +template +constexpr typename std::common_type::type min(NumberL lval, NumberR rval) { return (lval < rval) ? lval : rval; } diff --git a/tests/detail/numeric_util_test_suite.cpp b/tests/detail/numeric_util_test_suite.cpp index 2e9489fb..a6bf810a 100644 --- a/tests/detail/numeric_util_test_suite.cpp +++ b/tests/detail/numeric_util_test_suite.cpp @@ -31,6 +31,9 @@ public: { register_test(test_float_equals_zero); register_test(test_float_equals_large); + register_test(test_min); + register_test(test_max); + register_test(test_abs); } void test_float_equals_zero() @@ -108,5 +111,69 @@ public: // all the same options are available for increasing/decreasing the precision of the comparison // however the the epsilon source should always be of equal or lesser precision than the arguments when away from zero } + + void test_float_equals_nan() + { + const float nan = std::nan(""); + // nans always compare false + xlnt_assert(!xlnt::detail::float_equals(nan, 0.f)); + xlnt_assert(!xlnt::detail::float_equals(nan, nan)); + xlnt_assert(!xlnt::detail::float_equals(nan, 1000.f)); + } + + void test_min() + { + // simple + xlnt_assert(xlnt::detail::min(0, 1) == 0); + xlnt_assert(xlnt::detail::min(1, 0) == 0); + xlnt_assert(xlnt::detail::min(0.0, 1) == 0.0); // comparisons between different types just work + xlnt_assert(xlnt::detail::min(1, 0.0) == 0.0); + // negative numbers + xlnt_assert(xlnt::detail::min(0, -1) == -1.0); + xlnt_assert(xlnt::detail::min(-1, 0) == -1.0); + xlnt_assert(xlnt::detail::min(0.0, -1) == -1.0); + xlnt_assert(xlnt::detail::min(-1, 0.0) == -1.0); + // no zeroes + xlnt_assert(xlnt::detail::min(10, -10) == -10.0); + xlnt_assert(xlnt::detail::min(-10, 10) == -10.0); + xlnt_assert(xlnt::detail::min(10.0, -10) == -10.0); + xlnt_assert(xlnt::detail::min(-10, 10.0) == -10.0); + + static_assert(xlnt::detail::min(-10, 10.0) == -10.0, "constexpr"); + } + + void test_max() + { + // simple + xlnt_assert(xlnt::detail::max(0, 1) == 1); + xlnt_assert(xlnt::detail::max(1, 0) == 1); + xlnt_assert(xlnt::detail::max(0.0, 1) == 1.0); // comparisons between different types just work + xlnt_assert(xlnt::detail::max(1, 0.0) == 1.0); + // negative numbers + xlnt_assert(xlnt::detail::max(0, -1) == 0.0); + xlnt_assert(xlnt::detail::max(-1, 0) == 0.0); + xlnt_assert(xlnt::detail::max(0.0, -1) == 0.0); + xlnt_assert(xlnt::detail::max(-1, 0.0) == 0.0); + // no zeroes + xlnt_assert(xlnt::detail::max(10, -10) == 10.0); + xlnt_assert(xlnt::detail::max(-10, 10) == 10.0); + xlnt_assert(xlnt::detail::max(10.0, -10) == 10.0); + xlnt_assert(xlnt::detail::max(-10, 10.0) == 10.0); + + static_assert(xlnt::detail::max(-10, 10.0) == 10.0, "constexpr"); + } + + void test_abs() + { + xlnt_assert(xlnt::detail::abs(0) == 0); + xlnt_assert(xlnt::detail::abs(1) == 1); + xlnt_assert(xlnt::detail::abs(-1) == 1); + + xlnt_assert(xlnt::detail::abs(0.0) == 0.0); + xlnt_assert(xlnt::detail::abs(1.5) == 1.5); + xlnt_assert(xlnt::detail::abs(-1.5) == 1.5); + + static_assert(xlnt::detail::abs(-1.23) == 1.23, "constexpr"); + } }; static numeric_test_suite x; \ No newline at end of file From 875f143d7466453483a904824d0cd6cfe5cad0c2 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 25 Aug 2018 13:31:16 +1200 Subject: [PATCH 17/19] fix parameter ordering inconsistency that could cause switching the parameters to give different results --- source/detail/numeric_utils.hpp | 13 +++++++----- tests/detail/numeric_util_test_suite.cpp | 27 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/source/detail/numeric_utils.hpp b/source/detail/numeric_utils.hpp index 30f49f89..25d0dc67 100644 --- a/source/detail/numeric_utils.hpp +++ b/source/detail/numeric_utils.hpp @@ -88,18 +88,19 @@ bool float_equals(const LNumber &lhs, const RNumber &rhs, int epsilon_scale = 20) // scale the "fuzzy" equality. Higher value gives a more tolerant comparison { + // a type that lhs and rhs can agree on + using common_t = typename std::common_type::type; + // asserts for sane usage static_assert(std::is_floating_point::value || std::is_floating_point::value, "Using this function with two integers is just wasting time. Use =="); - static_assert(std::is_floating_point::value, - "Cannot extract epsilon from a number that isn't a floating point type"); + static_assert(std::numeric_limits::epsilon() < common_t{1}, + "epsilon >= 1.0 will cause all comparisons to return true"); // NANs always compare false with themselves if (std::isnan(lhs) || std::isnan(rhs)) { return false; } - // a type that lhs and rhs can agree on - using common_t = typename std::common_type::type; // epsilon type defaults to float because even if both args are a higher precision type // either or both could have been promoted by prior operations // if a higher precision is required, the template type can be changed @@ -108,7 +109,9 @@ float_equals(const LNumber &lhs, const RNumber &rhs, // epsilon for numeric_limits is valid when abs(x) <1.0, scaling only needs to be upwards // in particular, this prevents a lhs of 0 from requiring an exact comparison // additionally, a scale factor is applied. - common_t scaled_fuzz = epsilon_scale * epsilon * max(xlnt::detail::abs(lhs), common_t{1}); + common_t scaled_fuzz = epsilon_scale * epsilon * max(max(xlnt::detail::abs(lhs), + xlnt::detail::abs(rhs)), // |max| of parameters. + common_t{1}); // clamp return ((lhs + scaled_fuzz) >= rhs) && ((rhs + scaled_fuzz) >= lhs); } diff --git a/tests/detail/numeric_util_test_suite.cpp b/tests/detail/numeric_util_test_suite.cpp index a6bf810a..e79d507e 100644 --- a/tests/detail/numeric_util_test_suite.cpp +++ b/tests/detail/numeric_util_test_suite.cpp @@ -31,6 +31,7 @@ public: { register_test(test_float_equals_zero); register_test(test_float_equals_large); + register_test(test_float_equals_fairness); register_test(test_min); register_test(test_max); register_test(test_abs); @@ -121,6 +122,32 @@ public: xlnt_assert(!xlnt::detail::float_equals(nan, 1000.f)); } + void test_float_equals_fairness() + { + // tests for parameter ordering dependency + // (lhs ~= rhs) == (rhs ~= lhs) + const double test_val = 1.0; + const double test_diff_pass = 1.192092e-07; // should all pass with this + const double test_diff = 1.192093e-07; // difference enough to provide different results if the comparison is not "fair" + const double test_diff_fails = 1.192094e-07; // should all fail with this + + // test_diff_pass + xlnt_assert(xlnt::detail::float_equals((test_val + test_diff_pass), test_val, 1)); + xlnt_assert(xlnt::detail::float_equals(test_val, (test_val + test_diff_pass), 1)); + xlnt_assert(xlnt::detail::float_equals(-(test_val + test_diff_pass), -test_val, 1)); + xlnt_assert(xlnt::detail::float_equals(-test_val, -(test_val + test_diff_pass), 1)); + // test_diff + xlnt_assert(xlnt::detail::float_equals((test_val + test_diff), test_val, 1)); + xlnt_assert(xlnt::detail::float_equals(test_val, (test_val + test_diff), 1)); + xlnt_assert(xlnt::detail::float_equals(-(test_val + test_diff), -test_val, 1)); + xlnt_assert(xlnt::detail::float_equals(-test_val, -(test_val + test_diff), 1)); + // test_diff_fails + xlnt_assert(!xlnt::detail::float_equals((test_val + test_diff_fails), test_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(test_val, (test_val + test_diff_fails), 1)); + xlnt_assert(!xlnt::detail::float_equals(-(test_val + test_diff_fails), -test_val, 1)); + xlnt_assert(!xlnt::detail::float_equals(-test_val, -(test_val + test_diff_fails), 1)); + } + void test_min() { // simple From 1003ba507db56a0acff8d062da6593b2ae5c3ea6 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 25 Aug 2018 13:54:54 +1200 Subject: [PATCH 18/19] Add a test for serialize_number_to_string -- should allow serialisation of up to 15 significant digits -- don't use default stringstreams, typically only get ~6 significant digits --- tests/detail/numeric_util_test_suite.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/detail/numeric_util_test_suite.cpp b/tests/detail/numeric_util_test_suite.cpp index e79d507e..3df14799 100644 --- a/tests/detail/numeric_util_test_suite.cpp +++ b/tests/detail/numeric_util_test_suite.cpp @@ -29,6 +29,7 @@ class numeric_test_suite : public test_suite public: numeric_test_suite() { + register_test(test_serialise_number); register_test(test_float_equals_zero); register_test(test_float_equals_large); register_test(test_float_equals_fairness); @@ -37,6 +38,22 @@ public: register_test(test_abs); } + void test_serialise_number() + { + // excel serialises numbers as floating point values with <= 15 digits of precision + xlnt_assert(xlnt::detail::serialize_number_to_string(1) == "1"); + // trailing zeroes are ignored + xlnt_assert(xlnt::detail::serialize_number_to_string(1.0) == "1"); + xlnt_assert(xlnt::detail::serialize_number_to_string(1.0f) == "1"); + // one to 1 relation + xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456) == "1.23456"); + xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456789012345) == "1.23456789012345"); + xlnt_assert(xlnt::detail::serialize_number_to_string(123456.789012345) == "123456.789012345"); + xlnt_assert(xlnt::detail::serialize_number_to_string(1234567890123.45) == "1234567890123.45"); + // extra precision gets trimmed + xlnt_assert(xlnt::detail::serialize_number_to_string(1.234567890123456) == "1.23456789012345"); + } + void test_float_equals_zero() { // comparing relatively small numbers (2.3e-6) with 0 will be true by default From e183e37ae668aef3286228a26cd1ef9209a83d10 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 25 Aug 2018 14:06:20 +1200 Subject: [PATCH 19/19] remove roundoff test, add test for exponential representations -- testing that a number with >15 significant digits rounded off to 15 failed on all tested platforms --- tests/detail/numeric_util_test_suite.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/detail/numeric_util_test_suite.cpp b/tests/detail/numeric_util_test_suite.cpp index 3df14799..5fb033d1 100644 --- a/tests/detail/numeric_util_test_suite.cpp +++ b/tests/detail/numeric_util_test_suite.cpp @@ -49,9 +49,8 @@ public: xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456) == "1.23456"); xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456789012345) == "1.23456789012345"); xlnt_assert(xlnt::detail::serialize_number_to_string(123456.789012345) == "123456.789012345"); - xlnt_assert(xlnt::detail::serialize_number_to_string(1234567890123.45) == "1234567890123.45"); - // extra precision gets trimmed - xlnt_assert(xlnt::detail::serialize_number_to_string(1.234567890123456) == "1.23456789012345"); + xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456789012345e+67) == "1.23456789012345e+67"); + xlnt_assert(xlnt::detail::serialize_number_to_string(1.23456789012345e-67) == "1.23456789012345e-67"); } void test_float_equals_zero()