On 3 November 2013 11:30, Paolo Carlini wrote:
> On 11/03/2013 12:19 PM, Jonathan Wakely wrote:
>>
>> Yes, Paolo pointed out these are failing on 32-bit targets, I've got a
>> patch coming. Luc is there any reason not to just replace all those large
>> constants with 0x1234ABCD, which fits in a long on 32-bit targets?
>
> By the way, that's what I did/hacked in my local tree ;)
I needed some other changes, otherwise comparing optional<long>{} ==
0x1234ABCD doesn't compile, because 0x1234ABCD is an int and
optional<T> doesn't support comparisons with anything except T.
> Jon, I have got a bunch of other minor tweaks, from proper formatting of
> conditional operator and some curly braces, to using __and_ and __not_ when
> possible, and other thingies, like no full stops at the end of asserts and
> throws. Passes testing. You may want to integrate it with your other changes
> in testing... Or I can wait and apply it myself.
Here's the combined patch, tested x86_64-linux, 64-bit and 32-bit,
committed to trunk.
2013-11-05 Jonathan Wakely <jwakely....@gmail.com>
Paolo Carlini <paolo.carl...@oracle.com>
* include/experimental/optional: Use __and_<> and __not_<> in
conditions. Style fixes.
(__constexpr_addressof, swap): Make inline.
* testsuite/experimental/optional/cons/copy.cc: Adjust constants for
32-bit targets.
* testsuite/experimental/optional/cons/move.cc: Likewise.
* testsuite/experimental/optional/cons/value.cc: Likewise.
* testsuite/experimental/optional/constexpr/cons/value.cc: Likewise.
diff --git a/libstdc++-v3/include/experimental/optional
b/libstdc++-v3/include/experimental/optional
index 5915892..5882ff5 100644
--- a/libstdc++-v3/include/experimental/optional
+++ b/libstdc++-v3/include/experimental/optional
@@ -129,7 +129,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Tp>
struct _Has_addressof_impl<_Tp,
decltype( std::declval<const _Tp&>().operator&(), void() )>
- : std::true_type { };
+ : std::true_type { };
/**
* @brief Trait that detects the presence of an overloaded unary operator&.
@@ -157,7 +157,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*/
template<typename _Tp, typename enable_if<_Has_addressof<_Tp>::value,
int>::type...>
- _Tp* __constexpr_addressof(_Tp& __t)
+ inline _Tp* __constexpr_addressof(_Tp& __t)
{ return std::__addressof(__t); }
/**
@@ -235,32 +235,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (this->_M_engaged && __other._M_engaged)
this->_M_get() = __other._M_get();
else
- {
- if (__other._M_engaged)
- this->_M_construct(__other._M_get());
- else
- this->_M_reset();
- }
+ {
+ if (__other._M_engaged)
+ this->_M_construct(__other._M_get());
+ else
+ this->_M_reset();
+ }
return *this;
}
_Optional_base&
operator=(_Optional_base&& __other)
- noexcept(is_nothrow_move_constructible<_Tp>()
- && is_nothrow_move_assignable<_Tp>())
+ noexcept(__and_<is_nothrow_move_constructible<_Tp>,
+ is_nothrow_move_assignable<_Tp>>())
{
- if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = std::move(__other._M_get());
- else
- {
- if (__other._M_engaged)
- this->_M_construct(std::move(__other._M_get()));
- else
- this->_M_reset();
- }
-
- return *this;
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = std::move(__other._M_get());
+ else
+ {
+ if (__other._M_engaged)
+ this->_M_construct(std::move(__other._M_get()));
+ else
+ this->_M_reset();
+ }
+ return *this;
}
// [X.Y.4.2] Destructor.
@@ -373,35 +372,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Optional_base&
operator=(const _Optional_base& __other)
{
- if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = __other._M_get();
- else
- {
- if (__other._M_engaged)
- this->_M_construct(__other._M_get());
- else
- this->_M_reset();
- }
-
- return *this;
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = __other._M_get();
+ else
+ {
+ if (__other._M_engaged)
+ this->_M_construct(__other._M_get());
+ else
+ this->_M_reset();
+ }
+ return *this;
}
_Optional_base&
operator=(_Optional_base&& __other)
- noexcept(is_nothrow_move_constructible<_Tp>()
- && is_nothrow_move_assignable<_Tp>())
+ noexcept(__and_<is_nothrow_move_constructible<_Tp>,
+ is_nothrow_move_assignable<_Tp>>())
{
- if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = std::move(__other._M_get());
- else
- {
- if (__other._M_engaged)
- this->_M_construct(std::move(__other._M_get()));
- else
- this->_M_reset();
- }
-
- return *this;
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = std::move(__other._M_get());
+ else
+ {
+ if (__other._M_engaged)
+ this->_M_construct(std::move(__other._M_get()));
+ else
+ this->_M_reset();
+ }
+ return *this;
}
// Sole difference
@@ -445,9 +442,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
private:
struct _Empty_byte { };
- union {
- _Empty_byte _M_empty;
- _Stored_type _M_payload;
+ union
+ {
+ _Empty_byte _M_empty;
+ _Stored_type _M_payload;
};
bool _M_engaged = false;
};
@@ -462,22 +460,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Copy constructor.
is_copy_constructible<_Tp>::value,
// Copy assignment.
- is_copy_constructible<_Tp>::value
- && is_copy_assignable<_Tp>::value,
+ __and_<is_copy_constructible<_Tp>, is_copy_assignable<_Tp>>::value,
// Move constructor.
is_move_constructible<_Tp>::value,
// Move assignment.
- is_move_constructible<_Tp>::value
- && is_move_assignable<_Tp>::value,
+ __and_<is_move_constructible<_Tp>, is_move_assignable<_Tp>>::value,
// Unique tag type.
optional<_Tp>>
{
- static_assert(!is_same<typename remove_cv<_Tp>::type,
- nullopt_t>()
- && !is_same<typename remove_cv<_Tp>::type,
- in_place_t>()
- && !is_reference<_Tp>(),
- "Invalid instantiation of optional<T>.");
+ static_assert(__and_<__not_<is_same<typename remove_cv<_Tp>::type,
+ nullopt_t>>,
+ __not_<is_same<typename remove_cv<_Tp>::type,
+ in_place_t>>,
+ __not_<is_reference<_Tp>>>(),
+ "Invalid instantiation of optional<T>");
private:
using _Base = _Optional_base<_Tp>;
@@ -503,9 +499,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>::type
operator=(_Up&& __u)
{
- static_assert(is_constructible<_Tp, _Up>()
- && is_assignable<_Tp&, _Up>(),
- "Cannot assign to value type from argument.");
+ static_assert(__and_<is_constructible<_Tp, _Up>,
+ is_assignable<_Tp&, _Up>>(),
+ "Cannot assign to value type from argument");
if (this->_M_is_engaged())
this->_M_get() = std::forward<_Up>(__u);
@@ -520,7 +516,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
emplace(_Args&&... __args)
{
static_assert(is_constructible<_Tp, _Args&&...>(),
- "Cannot emplace value type from arguments.");
+ "Cannot emplace value type from arguments");
this->_M_reset();
this->_M_construct(std::forward<_Args>(__args)...);
@@ -551,15 +547,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (this->_M_is_engaged() && __other._M_is_engaged())
swap(this->_M_get(), __other._M_get());
else if (this->_M_is_engaged())
- {
- __other._M_construct(std::move(this->_M_get()));
- this->_M_destruct();
- }
+ {
+ __other._M_construct(std::move(this->_M_get()));
+ this->_M_destruct();
+ }
else if (__other._M_is_engaged())
- {
- this->_M_construct(std::move(__other._M_get()));
- __other._M_destruct();
- }
+ {
+ this->_M_construct(std::move(__other._M_get()));
+ __other._M_destruct();
+ }
}
// [X.Y.4.5] Observers.
@@ -585,11 +581,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr const _Tp&
value() const
{
- return this->_M_is_engaged() ?
- this->_M_get() :
- (__throw_bad_optional_access("Attempt to access value of a
disengaged"
- " optional object."),
- this->_M_get());
+ return this->_M_is_engaged()
+ ? this->_M_get()
+ : (__throw_bad_optional_access("Attempt to access value of a "
+ "disengaged optional object"),
+ this->_M_get());
}
_Tp&
@@ -598,34 +594,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (this->_M_is_engaged())
return this->_M_get();
- __throw_bad_optional_access("Attempt to access value of a disengaged"
- " optional object.");
+ __throw_bad_optional_access("Attempt to access value of a "
+ "disengaged optional object");
}
template<typename _Up>
constexpr _Tp
value_or(_Up&& __u) const&
{
- static_assert(is_copy_constructible<_Tp>()
- && is_convertible<_Up&&, _Tp>(),
- "Cannot return value.");
+ static_assert(__and_<is_copy_constructible<_Tp>,
+ is_convertible<_Up&&, _Tp>>(),
+ "Cannot return value");
- return this->_M_is_engaged() ?
- this->_M_get() :
- static_cast<_Tp>(std::forward<_Up>(__u));
+ return this->_M_is_engaged()
+ ? this->_M_get()
+ : static_cast<_Tp>(std::forward<_Up>(__u));
}
template<typename _Up>
_Tp
value_or(_Up&& __u) &&
{
- static_assert( is_move_constructible<_Tp>()
- && is_convertible<_Up&&, _Tp>(),
- "Cannot return value." );
+ static_assert(__and_<is_move_constructible<_Tp>,
+ is_convertible<_Up&&, _Tp>>(),
+ "Cannot return value" );
- return this->_M_is_engaged() ?
- std::move(this->_M_get()) :
- static_cast<_Tp>(std::forward<_Up>(__u));
+ return this->_M_is_engaged()
+ ? std::move(this->_M_get())
+ : static_cast<_Tp>(std::forward<_Up>(__u));
}
};
@@ -635,7 +631,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
operator==(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
{
return static_cast<bool>(__lhs) == static_cast<bool>(__rhs)
- && (!__lhs || *__lhs == *__rhs);
+ && (!__lhs || *__lhs == *__rhs);
}
template<typename _Tp>
@@ -647,8 +643,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr bool
operator<(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
{
- return static_cast<bool>(__rhs)
- && (!__lhs || *__lhs < *__rhs);
+ return static_cast<bool>(__rhs) && (!__lhs || *__lhs < *__rhs);
}
template<typename _Tp>
@@ -790,7 +785,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// [X.Y.11]
template<typename _Tp>
- void
+ inline void
swap(optional<_Tp>& __lhs, optional<_Tp>& __rhs)
noexcept(noexcept(__lhs.swap(__rhs)))
{ __lhs.swap(__rhs); }
diff --git a/libstdc++-v3/testsuite/experimental/optional/cons/copy.cc
b/libstdc++-v3/testsuite/experimental/optional/cons/copy.cc
index bbd9328..53c23a1 100644
--- a/libstdc++-v3/testsuite/experimental/optional/cons/copy.cc
+++ b/libstdc++-v3/testsuite/experimental/optional/cons/copy.cc
@@ -63,11 +63,12 @@ int main()
}
{
- std::experimental::optional<long> o { std::experimental::in_place,
0x1234ABCDF1E2D3C4 };
+ const long val = 0x1234ABCD;
+ std::experimental::optional<long> o { std::experimental::in_place, val};
auto copy = o;
VERIFY( copy );
- VERIFY( *copy == 0x1234ABCDF1E2D3C4 );
- VERIFY( o && o == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *copy == val );
+ VERIFY( o && o == val );
}
{
diff --git a/libstdc++-v3/testsuite/experimental/optional/cons/move.cc
b/libstdc++-v3/testsuite/experimental/optional/cons/move.cc
index 75d7677..ba31699 100644
--- a/libstdc++-v3/testsuite/experimental/optional/cons/move.cc
+++ b/libstdc++-v3/testsuite/experimental/optional/cons/move.cc
@@ -63,11 +63,12 @@ int main()
}
{
- std::experimental::optional<long> o { std::experimental::in_place,
0x1234ABCDF1E2D3C4 };
+ const long val = 0x1234ABCD;
+ std::experimental::optional<long> o { std::experimental::in_place, val};
auto moved_to = std::move(o);
VERIFY( moved_to );
- VERIFY( *moved_to == 0x1234ABCDF1E2D3C4 );
- VERIFY( o && *o == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *moved_to == val );
+ VERIFY( o && *o == val );
}
{
diff --git a/libstdc++-v3/testsuite/experimental/optional/cons/value.cc
b/libstdc++-v3/testsuite/experimental/optional/cons/value.cc
index 339c120..1eba62d 100644
--- a/libstdc++-v3/testsuite/experimental/optional/cons/value.cc
+++ b/libstdc++-v3/testsuite/experimental/optional/cons/value.cc
@@ -66,51 +66,51 @@ int main()
// [20.5.4.1] Constructors
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o { i };
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o = i;
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o = { i };
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o { std::move(i) };
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o = std::move(i);
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
- auto i = 0x1234ABCDF1E2D3C4;
+ auto i = 0x1234ABCD;
std::experimental::optional<long> o = { std::move(i) };
VERIFY( o );
- VERIFY( *o == 0x1234ABCDF1E2D3C4 );
- VERIFY( i == 0x1234ABCDF1E2D3C4 );
+ VERIFY( *o == 0x1234ABCD );
+ VERIFY( i == 0x1234ABCD );
}
{
diff --git
a/libstdc++-v3/testsuite/experimental/optional/constexpr/cons/value.cc
b/libstdc++-v3/testsuite/experimental/optional/constexpr/cons/value.cc
index 98e0a22..eb7233b 100644
--- a/libstdc++-v3/testsuite/experimental/optional/constexpr/cons/value.cc
+++ b/libstdc++-v3/testsuite/experimental/optional/constexpr/cons/value.cc
@@ -26,44 +26,44 @@ int main()
// [20.5.4.1] Constructors
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o { i };
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o = i;
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o = { i };
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o { std::move(i) };
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o = std::move(i);
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
{
- constexpr auto i = 0x1234ABCDF1E2D3C4;
+ constexpr long i = 0x1234ABCD;
constexpr std::experimental::optional<long> o = { std::move(i) };
static_assert( o, "" );
- static_assert( *o == 0x1234ABCDF1E2D3C4, "" );
+ static_assert( *o == 0x1234ABCD, "" );
}
}
