Skip to content

R9 of paper which fixes the reference stealing bug #103

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

Merged
steve-downey merged 10 commits into from
Jan 21, 2025
Merged

R9 of paper which fixes the reference stealing bug #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
34369eb
Make base.pdf compile
steve-downey Jan 11, 2025
d01f4ea
add Beman to bibliography
steve-downey Jan 11, 2025
28588cf
Add new constructors and assignments and reorder
steve-downey Jan 11, 2025
41ac3b7
Update verbiage around wording
steve-downey Jan 11, 2025
5637421
Improve parallelism in implementation
steve-downey Jan 11, 2025
daee5de
Rebase on current draft
steve-downey Jan 12, 2025
079b692
Clarify problem with copy and assign
steve-downey Jan 13, 2025
1babfd9
Fix reference problems
steve-downey Jan 13, 2025
c854b3b
Merge branch 'main' into refsteal-paper
steve-downey Jan 18, 2025
d481100
Lint fix -- newline and EOF
steve-downey Jan 18, 2025
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
77 changes: 41 additions & 36 deletions include/beman/optional26/optional.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,16 @@ concept enable_forward_value = !std::is_same_v<std::decay_t<U>, optional<T>> &&
!std::is_same_v<std::decay_t<U>, in_place_t> && std::is_constructible_v<T, U&&>;

template <class T, class U, class Other>
concept enable_from_other =
!std::is_same_v<T, U> && std::is_constructible_v<T, Other> && !std::is_constructible_v<T, optional<U>&> &&
!std::is_constructible_v<T, optional<U>&&> && !std::is_constructible_v<T, const optional<U>&> &&
!std::is_constructible_v<T, const optional<U>&&> && !std::is_convertible_v<optional<U>&, T> &&
!std::is_convertible_v<optional<U>&&, T> && !std::is_convertible_v<const optional<U>&, T> &&
!std::is_convertible_v<const optional<U>&&, T>;
concept enable_from_other = !std::is_same_v<T, U> && //
std::is_constructible_v<T, Other> && //
!std::is_constructible_v<T, optional<U>&> && //
!std::is_constructible_v<T, optional<U>&&> && //
!std::is_constructible_v<T, const optional<U>&> && //
!std::is_constructible_v<T, const optional<U>&&> && //
!std::is_convertible_v<optional<U>&, T> && //
!std::is_convertible_v<optional<U>&&, T> && //
!std::is_convertible_v<const optional<U>&, T> && //
!std::is_convertible_v<const optional<U>&&, T>;

template <class T, class U>
concept enable_assign_forward = !std::is_same_v<optional<T>, std::decay_t<U>> &&
Expand Down Expand Up @@ -279,12 +283,12 @@ class optional {
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, const U&>);

template <class U>
constexpr explicit(!std::is_convertible_v<U, T>) optional(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&>);
constexpr explicit(!std::is_convertible_v<U, T>) optional(const optional<U>& rhs)
requires(std::is_reference_v<U> && detail::enable_from_other<T, U, U>);

template <class U>
constexpr explicit(!std::is_convertible_v<U&, T>) optional(const optional<U&>& rhs)
requires(detail::enable_from_other<T, U&, U&>);
constexpr explicit(!std::is_convertible_v<U, T>) optional(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&>);

// \ref{optional.dtor}, destructor
constexpr ~optional()
Expand Down Expand Up @@ -324,12 +328,12 @@ class optional {
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, const U&>);

template <class U>
constexpr optional& operator=(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>);
constexpr optional& operator=(const optional<U>& rhs)
requires(std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>);

template <class U>
constexpr optional& operator=(const optional<U&>& rhs)
requires(detail::enable_assign_from_other<T, U&, U&>);
constexpr optional& operator=(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>);

template <class... Args>
constexpr T& emplace(Args&&... args);
Expand Down Expand Up @@ -358,9 +362,9 @@ class optional {
constexpr T& value() &;
constexpr const T& value() const&;
constexpr T&& value() &&;
template <class U>
template <class U = std::remove_cv_t<T>>
constexpr T value_or(U&& u) const&;
template <class U>
template <class U = std::remove_cv_t<T>>
constexpr T value_or(U&& u) &&;

// \ref{optional.monadic}, monadic operations
Expand Down Expand Up @@ -456,7 +460,7 @@ inline constexpr optional<T>::optional(in_place_t, std::initializer_list<U> il,
template <class T>
template <class U>
inline constexpr optional<T>::optional(U&& u)
requires detail::enable_forward_value<T, U> //&& std::is_convertible_v<U&&, T>
requires detail::enable_forward_value<T, U>
: optional(in_place, std::forward<U>(u)) {}

/// Converting copy constructor.
Expand All @@ -470,24 +474,25 @@ inline constexpr optional<T>::optional(const optional<U>& rhs)
}
}

/// Converting move constructor.
/// Converting copy constructor for U&
Copy link
Member

@neatudarius neatudarius Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please add some tests to cover this bugfix? Thanks for fixing it!

template <class T>
template <class U>
inline constexpr optional<T>::optional(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&>)
inline constexpr optional<T>::optional(const optional<U>& rhs)
requires(std::is_reference_v<U> && detail::enable_from_other<T, U, U>)
{
if (rhs.has_value()) {
construct(std::move(*rhs));
construct(*rhs);
}
}

/// Converting move constructor.
template <class T>
template <class U>
inline constexpr optional<T>::optional(const optional<U&>& rhs)
requires(detail::enable_from_other<T, U&, U&>)
inline constexpr optional<T>::optional(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_from_other<T, U, U &&>)
{
if (rhs.has_value()) {
construct(*rhs);
construct(std::move(*rhs));
}
}

Expand Down Expand Up @@ -578,45 +583,45 @@ inline constexpr optional<T>& optional<T>::operator=(const optional<U>& rhs)
return *this;
}

/// Converting move assignment operator.
///
/// Moves the value from `rhs` if there is one. Otherwise resets the stored
/// value in `*this`.
template <class T>
template <class U>
inline constexpr optional<T>& optional<T>::operator=(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>)
inline constexpr optional<T>& optional<T>::operator=(const optional<U>& rhs)
requires(std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>)
{
if (has_value()) {
if (rhs.has_value()) {
value_ = std::move(*rhs);
value_ = *rhs;
} else {
hard_reset();
}
}

else if (rhs.has_value()) {
construct(std::move(*rhs));
construct(*rhs);
}

return *this;
}

/// Converting move assignment operator.
///
/// Moves the value from `rhs` if there is one. Otherwise resets the stored
/// value in `*this`.
template <class T>
template <class U>
inline constexpr optional<T>& optional<T>::operator=(const optional<U&>& rhs)
requires(detail::enable_assign_from_other<T, U&, U&>)
inline constexpr optional<T>& optional<T>::operator=(optional<U>&& rhs)
requires(!std::is_reference_v<U> && detail::enable_assign_from_other<T, U, U>)
{
if (has_value()) {
if (rhs.has_value()) {
value_ = *rhs;
value_ = std::move(*rhs);
} else {
hard_reset();
}
}

else if (rhs.has_value()) {
construct(*rhs);
construct(std::move(*rhs));
}

return *this;
Expand Down
47 changes: 29 additions & 18 deletions papers/P2988/base-optional.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
\rSec0[utilities]{General utilities library}

\rSec1[optional]{Optional objects}

\rSec2[optional.general]{General}
Expand Down Expand Up @@ -88,7 +86,7 @@
constexpr void swap(optional<T>&, optional<T>&) noexcept(@\seebelow@);

template<class T>
constexpr optional<@\seebelow@> make_optional(T&&);
constexpr optional<decay_t<T>> make_optional(T&&);
template<class T, class... Args>
constexpr optional<T> make_optional(Args&&... args);
template<class T, class U, class... Args>
Expand Down Expand Up @@ -124,7 +122,7 @@
constexpr explicit optional(in_place_t, Args&&...);
template<class U, class... Args>
constexpr explicit optional(in_place_t, initializer_list<U>, Args&&...);
template<class U = T>
template<class U = remove_cv_t<T>>
constexpr explicit(@\seebelow@) optional(U&&);
template<class U>
constexpr explicit(@\seebelow@) optional(const optional<U>&);
Expand All @@ -138,7 +136,7 @@
constexpr optional& operator=(nullopt_t) noexcept;
constexpr optional& operator=(const optional&);
constexpr optional& operator=(optional&&) noexcept(@\seebelow@);
template<class U = T> constexpr optional& operator=(U&&);
template<class U = remove_cv_t<T>> constexpr optional& operator=(U&&);
template<class U> constexpr optional& operator=(const optional<U>&);
template<class U> constexpr optional& operator=(optional<U>&&);
template<class... Args> constexpr T& emplace(Args&&...);
Expand Down Expand Up @@ -166,8 +164,8 @@
constexpr T& value() &; // freestanding-deleted
constexpr T&& value() &&; // freestanding-deleted
constexpr const T&& value() const &&; // freestanding-deleted
template<class U> constexpr T value_or(U&&) const &;
template<class U> constexpr T value_or(U&&) &&;
template<class U = remove_cv_t<T>> constexpr T value_or(U&&) const &;
template<class U = remove_cv_t<T>> constexpr T value_or(U&&) &&;

// \ref{optional.monadic}, monadic operations
template<class F> constexpr auto and_then(F&& f) &;
Expand All @@ -185,7 +183,7 @@
constexpr void reset() noexcept;

private:
T *val; // \expos
T* val; // \expos
};

template<class T>
Expand All @@ -197,8 +195,7 @@
Any instance of \tcode{optional<T>} at any given time either contains a value or does not contain a value.
When an instance of \tcode{optional<T>} \defnx{contains a value}{contains a value!\idxcode{optional}},
it means that an object of type \tcode{T}, referred to as the optional object's \defnx{contained value}{contained value!\idxcode{optional}},
is allocated within the storage of the optional object.
Implementations are not permitted to use additional storage, such as dynamic memory, to allocate its contained value.
is nested within\iref{intro.object} the optional object.
When an object of type \tcode{optional<T>} is contextually converted to \tcode{bool},
the conversion returns \tcode{true} if the object contains a value;
otherwise the conversion returns \tcode{false}.
Expand Down Expand Up @@ -359,7 +356,7 @@

\indexlibraryctor{optional}%
\begin{itemdecl}
template<class U = T> constexpr explicit(@\seebelow@) optional(U&& v);
template<class U = remove_cv_t<T>> constexpr explicit(@\seebelow@) optional(U&& v);
\end{itemdecl}

\begin{itemdescr}
Expand Down Expand Up @@ -611,16 +608,18 @@

\indexlibrarymember{operator=}{optional}%
\begin{itemdecl}
template<class U = T> constexpr optional<T>& operator=(U&& v);
template<class U = remove_cv_t<T>> constexpr optional& operator=(U&& v);
\end{itemdecl}

\begin{itemdescr}
\pnum
\constraints
\tcode{is_same_v<remove_cvref_t<U>, optional>} is \tcode{false},
\tcode{conjunction_v<is_scalar<T>, is_same<T, decay_t<U>>>} is \tcode{false},
\tcode{is_constructible_v<T, U>} is \tcode{true}, and
\tcode{is_assignable_v<T\&, U>} is \tcode{true}.
\begin{itemize}
\item \tcode{is_same_v<remove_cvref_t<U>, optional>} is \tcode{false},
\item \tcode{conjunction_v<is_scalar<T>, is_same<T, decay_t<U>>>} is \tcode{false},
\item \tcode{is_constructible_v<T, U>} is \tcode{true}, and
\item \tcode{is_assignable_v<T\&, U>} is \tcode{true}.
\end{itemize}

\pnum
\effects
Expand Down Expand Up @@ -1046,7 +1045,7 @@

\indexlibrarymember{value_or}{optional}%
\begin{itemdecl}
template<class U> constexpr T value_or(U&& v) const &;
template<class U = remove_cv_t<T>> constexpr T value_or(U&& v) const &;
\end{itemdecl}

\begin{itemdescr}
Expand All @@ -1064,7 +1063,7 @@

\indexlibrarymember{value_or}{optional}%
\begin{itemdecl}
template<class U> constexpr T value_or(U&& v) &&;
template<class U = remove_cv_t<T>> constexpr T value_or(U&& v) &&;
\end{itemdecl}

\begin{itemdescr}
Expand Down Expand Up @@ -1512,6 +1511,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x == v} is well-formed and
its result is convertible to \tcode{bool}.
\begin{note}
Expand All @@ -1531,6 +1531,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v == *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1547,6 +1548,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x != v} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1563,6 +1565,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v != *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1579,6 +1582,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x < v} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1595,6 +1599,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v < *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1611,6 +1616,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x > v} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1627,6 +1633,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v > *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1643,6 +1650,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x <= v} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1659,6 +1667,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v <= *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1675,6 +1684,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{U} is not a specialization of \tcode{optional}.
The expression \tcode{*x >= v} is well-formed and
its result is convertible to \tcode{bool}.

Expand All @@ -1691,6 +1701,7 @@
\begin{itemdescr}
\pnum
\constraints
\tcode{T} is not a specialization of \tcode{optional}.
The expression \tcode{v >= *x} is well-formed and
its result is convertible to \tcode{bool}.

Expand Down
9 changes: 9 additions & 0 deletions papers/P2988/mybiblio.bib
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,12 @@ @misc{rawgithu58:online
year = {},
note = {(Accessed on 08/14/2024)}
}

@misc{The_Beman_Project_beman_optional26,
author = {The Beman Project},
license = {Apache-2.0},
title = {{beman.optional26}},
howpublished = {\url{https://github.com/bemanproject/optional26}},
Copy link
Member

@neatudarius neatudarius Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reminder here about renaming. Feel free to ignore it. Just trying to help for Austria.

month = {},
year = {},
}
Loading
Loading