Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixing memory leaks and reducing memory usage with unique_ptr #666

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixing memory leaks and reducing memory usage with unique_ptr #666

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3ec1477
Fixing memory leaks and reducing memory usage by switching to a uniqu…
ThibaultDECO Sep 8, 2022
1a22f68
Merge branch 'master' into patch-6
ThibaultDECO Oct 9, 2022
ea15e07
Update optional.hpp
ThibaultDECO Oct 9, 2022
42d336c
Merge branch 'master' into patch-6
tfussell Dec 3, 2022
45eadf5
Merge branch 'master' into patch-6
ThibaultDECO Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 48 additions & 106 deletions include/xlnt/utils/optional.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "xlnt/utils/numeric.hpp"
#include "xlnt/xlnt_config.hpp"
#include <type_traits>
#include <memory>

namespace xlnt {

Expand Down Expand Up @@ -74,55 +75,49 @@ class optional
/// Default contructor. is_set() will be false initially.
/// </summary>
optional() noexcept
: has_value_(false)
: optional_ptr()
{
}

/// <summary>
/// Constructs this optional with a value.
/// Copy constructs this optional with a value.
/// noexcept if T copy ctor is noexcept
/// </summary>
optional(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_copy_T_noexcept{}))
: has_value_(true)
: optional_ptr(new T(value))
{
new (&storage_) T(value);
}

/// <summary>
/// Constructs this optional with a value.
/// Move constructs this optional with a value.
/// noexcept if T move ctor is noexcept
/// </summary>
optional(T &&value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(ctor_move_T_noexcept{}))
: has_value_(true)
: optional_ptr(new T(std::move(value)))
{
new (&storage_) T(std::move(value));
}

/// <summary>
/// Copy constructs this optional from other
/// noexcept if T copy ctor is noexcept
/// </summary>
optional(const optional &other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(copy_ctor_noexcept{}))
: has_value_(other.has_value_)
{
if (has_value_)
if (other.optional_ptr != nullptr)
{
new (&storage_) T(other.value_ref());
}
optional_ptr = std::make_unique<T>(*other.optional_ptr);
} else {
optional_ptr = nullptr;
}
}

/// <summary>
/// Move constructs this optional from other. Clears the value from other if set
/// noexcept if T move ctor is noexcept
/// </summary>
optional(optional &&other) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(move_ctor_noexcept{}))
: has_value_(other.has_value_)
: optional_ptr(std::move(other.optional_ptr))
{
if (has_value_)
{
new (&storage_) T(std::move(other.value_ref()));
other.clear();
}
}

/// <summary>
Expand All @@ -131,14 +126,12 @@ class optional
/// </summary>
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
if (other.optional_ptr != nullptr)
{
clear();
}
optional_ptr.reset(new T(*other.optional_ptr));
} else {
optional_ptr = nullptr;
}
return *this;
}

Expand All @@ -148,15 +141,7 @@ class optional
/// </summary>
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();
}
optional_ptr = std::move(other.optional_ptr);
return *this;
}

Expand All @@ -165,7 +150,7 @@ class optional
/// </summary>
~optional() noexcept // note:: unconditional because msvc freaks out otherwise
{
clear();
optional_ptr.reset();
}

/// <summary>
Expand All @@ -174,65 +159,35 @@ class optional
/// </summary>
bool is_set() const noexcept
{
return has_value_;
return (optional_ptr != nullptr);
}

/// <summary>
/// Copies the value into the stored value
/// </summary>
void set(const T &value) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{}))
{
#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
optional_ptr.reset(new T(value));
}

/// <summary>
/// Moves the value into the stored value
/// </summary>
// 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
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
optional_ptr.reset(new T(std::move(value)));
}

/// <summary>
/// Assignment operator overload. Equivalent to setting the value using optional::set.
/// </summary>
optional &operator=(const T &rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_copy_noexcept_t{}))
{
set(rhs);
optional_ptr.reset(new T(rhs));
return *this;
}

Expand All @@ -241,7 +196,7 @@ class optional
/// </summary>
optional &operator=(T &&rhs) noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(set_move_noexcept_t{}))
{
set(std::move(rhs));
optional_ptr.reset(new T(std::move(rhs)));
return *this;
}

Expand All @@ -250,11 +205,7 @@ class optional
/// </summary>
void clear() noexcept(XLNT_NOEXCEPT_VALUE_COMPAT(clear_noexcept_t{}))
{
if (has_value_)
{
reinterpret_cast<T *>(&storage_)->~T();
}
has_value_ = false;
optional_ptr.reset();
}

/// <summary>
Expand All @@ -263,12 +214,11 @@ class optional
/// </summary>
T &get()
{
if (!has_value_)
if (optional_ptr == nullptr)
{
throw invalid_attribute();
}

return value_ref();
return *optional_ptr;
}

/// <summary>
Expand All @@ -277,12 +227,11 @@ class optional
/// </summary>
const T &get() const
{
if (!has_value_)
if (optional_ptr == nullptr)
{
throw invalid_attribute();
}

return value_ref();
return *optional_ptr;
}

/// <summary>
Expand All @@ -292,16 +241,21 @@ class optional
/// </summary>
bool operator==(const optional<T> &other) const noexcept
{
if (has_value_ != other.has_value_)
if (optional_ptr == nullptr)
{
return false;
}
if (!has_value_)
{
return true;
}
// equality is overloaded to provide fuzzy equality when T is a fp number
return compare_equal(value_ref(), other.value_ref());
if (other.optional_ptr == nullptr) {
return true;
} else {
return false;
};
} else {
if (other.optional_ptr == nullptr) {
return false;
} else {
// equality is overloaded to provide fuzzy equality when T is a fp number
return compare_equal(*optional_ptr, *other.optional_ptr);
};
};
}

/// <summary>
Expand All @@ -315,19 +269,7 @@ class optional
}

private:
// helpers for getting a T out of storage
T &value_ref() noexcept
{
return *reinterpret_cast<T *>(&storage_);
}

const T &value_ref() const noexcept
{
return *reinterpret_cast<const T *>(&storage_);
}

bool has_value_;
typename std::aligned_storage<sizeof(T), alignof(T)>::type storage_;
std::unique_ptr<T> optional_ptr;
};

#ifdef XLNT_NOEXCEPT_VALUE_COMPAT
Expand Down