diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp index 143c38ec..8248ebd2 100644 --- a/include/xlnt/utils/optional.hpp +++ b/include/xlnt/utils/optional.hpp @@ -25,6 +25,7 @@ #include #include +#include namespace xlnt { @@ -34,39 +35,207 @@ namespace xlnt { /// within the optional class. /// template -class XLNT_API optional +class optional { +#if defined(_MSC_VER) && _MSC_VER <= 1900 // v14, visual studio 2015 +#define XLNT_NOEXCEPT_VALUE_COMPAT(...) (false) +#else +#define XLNT_NOEXCEPT_VALUE_COMPAT(...) (__VA_ARGS__) + using ctor_copy_T_noexcept = typename std::conditional{}, std::true_type, std::false_type>::type; + using ctor_move_T_noexcept = typename std::conditional{}, std::true_type, std::false_type>::type; + using copy_ctor_noexcept = ctor_copy_T_noexcept; + using move_ctor_noexcept = ctor_move_T_noexcept; + using set_copy_noexcept_t = typename std::conditional{} && std::is_nothrow_assignable{}, std::true_type, std::false_type>::type; + 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 + public: /// /// Default contructor. is_set() will be false initially. /// - optional() : has_value_(false), value_(T()) + optional() noexcept + : has_value_(false) { } /// /// Constructs this optional with a value. + /// noexcept if T copy ctor is noexcept /// - optional(const T &value) : has_value_(true), value_(value) + optional(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_copy_T_noexcept{})) + : has_value_(true) { + new (&storage_) T(value); + } + + /// + /// Constructs this optional with a value. + /// noexcept if T move ctor is noexcept + /// + optional(T &&value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_move_T_noexcept{})) + : has_value_(true) + { + new (&storage_) T(std::move(value)); + } + + /// + /// Copy constructs this optional from other + /// noexcept if T copy ctor is noexcept + /// + optional(const optional &other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(copy_ctor_noexcept{})) + : has_value_(other.has_value_) + { + if (has_value_) + { + new (&storage_) T(other.value_ref()); + } + } + + /// + /// Move constructs this optional from other. Clears the value from other if set + /// noexcept if T move ctor is noexcept + /// + optional(optional &&other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(move_ctor_noexcept{})) + : has_value_(other.has_value_) + { + if (has_value_) + { + new (&storage_) T(std::move(other.value_ref())); + other.clear(); + } + } + + /// + /// Copy assignment of this optional from other + /// noexcept if set and clear are noexcept for T& + /// + optional &operator=(const optional &other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{} && clear_noexcept_t{})) + { + if (other.has_value_) + { + set(other.value_ref()); + } + else + { + clear(); + } + return *this; + } + + /// + /// Move assignment of this optional from other + /// noexcept if set and clear are noexcept for T&& + /// + optional &operator=(optional &&other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{} && clear_noexcept_t{})) + { + if (other.has_value_) + { + set(std::move(other.value_ref())); + other.clear(); + } + else + { + clear(); + } + return *this; + } + + /// + /// Destructor cleans up the T instance if set + /// + ~optional() noexcept // note:: unconditional because msvc freaks out otherwise + { + clear(); } /// /// Returns true if this object currently has a value set. This should /// be called before accessing the value with optional::get(). /// - bool is_set() const + bool is_set() const noexcept { return has_value_; } /// - /// Sets the value to value. + /// Copies the value into the stored value /// - void set(const T &value) + void set(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{})) { - has_value_ = true; - value_ = value; +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif + if (has_value_) + { + value_ref() = value; + } + else + { + new (&storage_) T(value); + has_value_ = true; + } +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#endif + } + + /// + /// Moves the value into the stored value + /// + void set(T &&value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{})) + { + // note seperate overload for two reasons (as opposed to perfect forwarding) + // 1. have to deal with implicit conversions internally with perfect forwarding + // 2. have to deal with the noexcept specfiers for all the different variations + // overload is just far and away the simpler solution +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif + if (has_value_) + { + value_ref() = std::move(value); + } + else + { + new (&storage_) T(std::move(value)); + has_value_ = true; + } +#if defined(__GNUC__) && !defined(__clang__) +#pragma GCC diagnostic pop +#endif + } + + /// + /// Assignment operator overload. Equivalent to setting the value using optional::set. + /// + optional &operator=(const T &rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{})) + { + set(rhs); + return *this; + } + + /// + /// Assignment operator overload. Equivalent to setting the value using optional::set. + /// + optional &operator=(T &&rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{})) + { + set(std::move(rhs)); + return *this; + } + + /// + /// After this is called, is_set() will return false until a new value is provided. + /// + void clear() noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(clear_noexcept_t{})) + { + if (has_value_) + { + reinterpret_cast(&storage_)->~T(); + } + has_value_ = false; } /// @@ -80,7 +249,7 @@ public: throw invalid_attribute(); } - return value_; + return value_ref(); } /// @@ -94,28 +263,7 @@ public: throw invalid_attribute(); } - return value_; - } - - /// - /// Resets the internal value using its default constructor. After this is - /// called, is_set() will return false until a new value is provided. - /// - void clear() - { - has_value_ = false; - value_ = T(); - } - - /// - /// Assignment operator. Equivalent to setting the value using optional::set. - /// - optional &operator=(const T &rhs) - { - has_value_ = true; - value_ = rhs; - - return *this; + return value_ref(); } /// @@ -123,21 +271,47 @@ public: /// or both have a value and those values are equal according to /// their equality operator. /// - bool operator==(const optional &other) const + bool operator==(const optional &other) const noexcept { - if (has_value_ != other.has_value_) { + if (has_value_ != other.has_value_) + { return false; } if (!has_value_) { return true; } - return value_ == other.value_; + return value_ref() == other.value_ref(); + } + + /// + /// Returns false if neither this nor other have a value + /// or both have a value and those values are equal according to + /// their equality operator. + /// + bool operator!=(const optional &other) const noexcept + { + return !operator==(other); } private: + // helpers for getting a T out of storage + T &value_ref() noexcept + { + return *reinterpret_cast(&storage_); + } + + const T &value_ref() const noexcept + { + return *reinterpret_cast(&storage_); + } + bool has_value_; - T value_; + typename std::aligned_storage::type storage_; }; -} // namespace xlnt +#ifdef XLNT_NOEXCEPT_VALUE_COMPAT +#undef XLNT_NOEXCEPT_VALUE_COMPAT +#endif + +} // namespace xlnt \ No newline at end of file diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 2ea048d3..5c924e84 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -41,6 +41,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-padded") # ignore padding warnings 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 diff --git a/source/detail/serialization/xlsx_consumer.cpp b/source/detail/serialization/xlsx_consumer.cpp index 336ddaa3..a634bf7b 100644 --- a/source/detail/serialization/xlsx_consumer.cpp +++ b/source/detail/serialization/xlsx_consumer.cpp @@ -387,7 +387,7 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id) props.sync_horizontal.set(parser().attribute("syncHorizontal")); } if (parser().attribute_present("syncVertical")) - {// optional, boolean, false + { // optional, boolean, false props.sync_vertical.set(parser().attribute("syncVertical")); } if (parser().attribute_present("syncRef")) @@ -399,11 +399,11 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id) props.transition_evaluation.set(parser().attribute("transitionEvaluation")); } if (parser().attribute_present("transitionEntry")) - {// optional, boolean, false + { // optional, boolean, false props.transition_entry.set(parser().attribute("transitionEntry")); } if (parser().attribute_present("published")) - {// optional, boolean, true + { // optional, boolean, true props.published.set(parser().attribute("published")); } if (parser().attribute_present("codeName")) @@ -411,11 +411,11 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id) props.code_name.set(parser().attribute("codeName")); } if (parser().attribute_present("filterMode")) - {// optional, boolean, false + { // optional, boolean, false props.filter_mode.set(parser().attribute("filterMode")); } if (parser().attribute_present("enableFormatConditionsCalculation")) - {// optional, boolean, true + { // optional, boolean, true props.enable_format_condition_calculation.set(parser().attribute("enableFormatConditionsCalculation")); } ws.d_->sheet_properties_.set(props); @@ -606,19 +606,22 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id) auto min = static_cast(std::stoull(parser().attribute("min"))); auto max = static_cast(std::stoull(parser().attribute("max"))); - optional width; - - if (parser().attribute_present("width")) - { - width = (parser().attribute("width") * 7 - 5) / 7; - } - - optional column_style; - - if (parser().attribute_present("style")) - { - column_style = parser().attribute("style"); - } + // avoid uninitialised warnings in GCC by using a lambda to make the conditional initialisation + optional width = [](xml::parser &p) -> xlnt::optional { + if (p.attribute_present("width")) + { + return (p.attribute("width") * 7 - 5) / 7; + } + return xlnt::optional(); + }(parser()); + // avoid uninitialised warnings in GCC by using a lambda to make the conditional initialisation + optional column_style = [](xml::parser &p) -> xlnt::optional { + if (p.attribute_present("style")) + { + return p.attribute("style"); + } + return xlnt::optional(); + }(parser()); auto custom = parser().attribute_present("customWidth") ? is_true(parser().attribute("customWidth")) @@ -1843,7 +1846,8 @@ void xlsx_consumer::read_office_document(const std::string &content_type) // CT_ target_.d_->sheet_title_rel_id_map_.end(), [&](const std::pair &p) { return p.second == worksheet_rel.id(); - })->first; + }) + ->first; auto id = sheet_title_id_map_[title]; auto index = sheet_title_index_map_[title]; diff --git a/tests/utils/optional_tests.cpp b/tests/utils/optional_tests.cpp new file mode 100644 index 00000000..6313e167 --- /dev/null +++ b/tests/utils/optional_tests.cpp @@ -0,0 +1,307 @@ +// 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 +#include +#include + +// test helpers +namespace { +// increments count when constructed, decrements when destructed +// use to ensure correct ctor/dtor pairing +struct alive_count +{ + alive_count() + { + ++count; + } + + alive_count(const alive_count &) + { + ++count; + } + + alive_count(alive_count &&) + { + ++count; + } + + ~alive_count() + { + --count; + } + + alive_count &operator=(const alive_count &) = default; + alive_count &operator=(alive_count &&) = default; + + static int count; +}; +int alive_count::count = 0; + +// implicitly convertible from int +struct convertible +{ + // implicit construction from int + convertible(int i) + : val(i) + { + } + + int val; +}; + +// default ctor deleted +struct no_default +{ + no_default() = delete; + int i; +}; +} // namespace + +class optional_test_suite : public test_suite +{ +public: + optional_test_suite() + : test_suite() + { + register_test(test_ctor); + register_test(test_copy_ctor); + register_test(test_move_ctor); + register_test(test_copy_assign); + register_test(test_move_assign); + register_test(test_set_and_get); + register_test(test_equality); + register_test(test_const); + } + + void test_ctor() + { + // default + xlnt::optional opt1; + xlnt_assert(!opt1.is_set()); + // value + const int test_val = 3; + xlnt::optional opt2(test_val); + xlnt_assert(opt2.is_set()); + xlnt_assert_equals(opt2.get(), test_val); + // converting + xlnt::optional opt3(test_val); + xlnt_assert(opt3.is_set()); + xlnt_assert_equals(opt3.get().val, test_val); + // no default ctor + xlnt::optional no_def_opt; + } + + void test_copy_ctor() + { + { // copy behaviour + xlnt::optional opt1; + xlnt::optional opt2(opt1); + xlnt_assert_equals(opt1, opt2); + + const int test_val = 123; + xlnt::optional opt3(test_val); + xlnt::optional opt4(opt3); + xlnt_assert_equals(opt3, opt4); + } + { // lifetime checks + xlnt::optional opt1(alive_count{}); + xlnt_assert_equals(1, alive_count::count); + { + xlnt::optional opt2(opt1); + xlnt_assert_equals(2, alive_count::count); + } + xlnt_assert_equals(1, alive_count::count); + } + xlnt_assert_equals(0, alive_count::count); // dtor test + } + + void test_move_ctor() + { + { // move behaviour + xlnt::optional opt1; + xlnt::optional opt2(std::move(opt1)); + xlnt_assert_equals(opt2, xlnt::optional{}); // can't test against opt1 so use a temporary + + const int test_val = 123; + xlnt::optional opt3(test_val); + xlnt::optional opt4(std::move(opt3)); + xlnt_assert(opt4.is_set()); // moved to optional contains the value + xlnt_assert_equals(opt4.get(), test_val); + } + { // lifetime checks + xlnt::optional opt1(alive_count{}); + xlnt_assert_equals(1, alive_count::count); + { + xlnt::optional opt2(std::move(opt1)); + xlnt_assert_equals(1, alive_count::count); // opt1 is in a no-value state + } + xlnt_assert_equals(0, alive_count::count); + } + xlnt_assert_equals(0, alive_count::count); // dtor test + } + + void test_copy_assign() + { + { // copy assign behaviour + xlnt::optional opt1; + xlnt::optional opt_assign1; // to actually test assignment, the value needs to be already created. using '=' is not enough + opt_assign1 = opt1; + xlnt_assert_equals(opt1, opt_assign1); + + const int test_val = 123; + xlnt::optional opt2(test_val); + xlnt::optional opt_assign2; + opt_assign2 = opt2; + xlnt_assert_equals(opt2, opt_assign2); + } + { // lifetime checks + xlnt::optional opt1(alive_count{}); + xlnt_assert_equals(1, alive_count::count); + { + xlnt::optional opt_assign1; + opt_assign1 = opt1; + xlnt_assert_equals(2, alive_count::count); + } + xlnt_assert_equals(1, alive_count::count); + } + xlnt_assert_equals(0, alive_count::count); // dtor test + } + + void test_move_assign() + { + { // copy assign behaviour + xlnt::optional opt1; + xlnt::optional opt_assign1; // to actually test assignment, the value needs to be already created. using '=' is not enough + opt_assign1 = std::move(opt1); + xlnt_assert_equals(opt_assign1, xlnt::optional{}); // can't test against opt1 so use a temporary + + const int test_val = 123; + xlnt::optional opt2(test_val); + xlnt::optional opt_assign2; + opt_assign2 = std::move(opt2); + xlnt_assert(opt_assign2.is_set()); // moved to optional contains the value + xlnt_assert_equals(opt_assign2.get(), test_val); + } + { // lifetime checks + xlnt::optional opt1(alive_count{}); + xlnt_assert_equals(1, alive_count::count); + { + xlnt::optional opt_assign1; + opt_assign1 = std::move(opt1); + xlnt_assert_equals(1, alive_count::count); // opt1 is in a no-value state + } + xlnt_assert_equals(0, alive_count::count); + } + xlnt_assert_equals(0, alive_count::count); // dtor test + } + + void test_set_and_get() + { + { + xlnt::optional test_opt; + xlnt_assert(!test_opt.is_set()); + xlnt_assert_throws(test_opt.get(), xlnt::invalid_attribute); + // set + const int test_val1 = 321; + test_opt.set(test_val1); + xlnt_assert(test_opt.is_set()); + xlnt_assert_equals(test_opt.get(), test_val1); + // set again + const int test_val2 = 123; + test_opt.set(test_val2); + xlnt_assert(test_opt.is_set()); + xlnt_assert_equals(test_opt.get(), test_val2); + // clear + test_opt.clear(); + xlnt_assert(!test_opt.is_set()); + xlnt_assert_throws(test_opt.get(), xlnt::invalid_attribute); + // set again + const int test_val3 = 3; + test_opt.set(test_val3); + xlnt_assert(test_opt.is_set()); + xlnt_assert_equals(test_opt.get(), test_val3); + // operator= set + xlnt::optional test_opt2; + test_opt2 = test_val1; + xlnt_assert_equals(test_opt2.get(), test_val1); + } + { // lifetime checks + xlnt::optional test_opt; + xlnt_assert_equals(0, alive_count::count); + test_opt.set(alive_count()); + xlnt_assert_equals(1, alive_count::count); + test_opt.set(alive_count()); // reassignment doesn't somehow skip the dtor + xlnt_assert_equals(1, alive_count::count); + test_opt.clear(); + xlnt_assert_equals(0, alive_count::count); + } + } + + void test_equality() + { + xlnt::optional test_opt1; + xlnt::optional test_opt2; + // no value opts compare equal + xlnt_assert(test_opt1 == test_opt2); + xlnt_assert(!(test_opt1 != test_opt2)); + xlnt_assert(test_opt2 == test_opt1); + xlnt_assert(!(test_opt2 != test_opt1)); + // value compares false with no value + const int test_val = 1; + test_opt1.set(test_val); + xlnt_assert(test_opt1 != test_opt2); + xlnt_assert(!(test_opt1 == test_opt2)); + xlnt_assert(test_opt2 != test_opt1); + xlnt_assert(!(test_opt2 == test_opt1)); + // value compares false with a different value + const int test_val2 = 2; + test_opt2.set(test_val2); + xlnt_assert(test_opt1 != test_opt2); + xlnt_assert(!(test_opt1 == test_opt2)); + xlnt_assert(test_opt2 != test_opt1); + xlnt_assert(!(test_opt2 == test_opt1)); + // value compares equal with same value + test_opt2.set(test_val); + xlnt_assert(test_opt1 == test_opt2); + xlnt_assert(!(test_opt1 != test_opt2)); + xlnt_assert(test_opt2 == test_opt1); + xlnt_assert(!(test_opt2 != test_opt1)); + } + + void test_const() + { + // functions on a const optional + const int test_val = 1; + const xlnt::optional opt(test_val); + xlnt_assert(opt.is_set()); + xlnt_assert(opt.get() == test_val); + + xlnt::optional opt2(test_val); + xlnt_assert(opt == opt2); + xlnt_assert(opt2 == opt); + xlnt_assert(!(opt != opt2)); + xlnt_assert(!(opt2 != opt)); + } +}; +static optional_test_suite x; \ No newline at end of file