LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 41577 - test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp has wrong assumption
Summary: test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cp...
Status: RESOLVED FIXED
Alias: None
Product: libc++
Classification: Unclassified
Component: All Bugs (show other bugs)
Version: unspecified
Hardware: All All
: P normal
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-24 05:08 PDT by Jonathan Wakely
Modified: 2019-04-24 19:16 PDT (History)
2 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2019-04-24 05:08:15 PDT
std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp

This test assumes that move construction is not constexpr, based on this old wording quoted in the test:

// constexpr optional(const optional<T>&& rhs);
//   If is_trivially_move_constructible_v<T> is true,
//    this constructor shall be a constexpr constructor.

That was replaced by https://wg21.link/p0602r4 and now it says:

"If is_trivially_move_constructible_v<T> is true, this constructor is trivial."

That means the constructor might still be constexpr even if it's non-trivial.

It's not clear to me whether the constructor is *required* to be constexpr, but I think the test is wrong to assume the move construction *can't* be constexpr (the test compiles using the std::optional from libstdc++, but 'lit' expects it to be ill-formed).
Comment 1 Marshall Clow (home) 2019-04-24 09:21:54 PDT
This may be a bug in our optional implementation. 

Given:

#include <optional>
#include <type_traits>
#include <cassert>

struct S {
    constexpr S()       : v_(0) {}
    constexpr S(int v)  : v_(v) {}
    constexpr S(const S  &rhs) : v_(rhs.v_) {} // not trivially copyable
    constexpr S(      S &&rhs) : v_(rhs.v_) {} // not trivially moveable
    int v_;
    };


int main(int, char**)
{
    static_assert (!std::is_trivially_move_constructible_v<S>, "");
	constexpr S s1(3);
	constexpr S s2 = std::move(s0);
	(void) s2;

    constexpr std::optional<S> o1;
    constexpr std::optional<S> o2 = std::move(o1);  // not constexpr
  return 0;
}

I don't see why the line marked "not constexpr" shouldn't be constexpr.
Comment 2 Marshall Clow (home) 2019-04-24 12:35:23 PDT
Better test:

#include <optional>
#include <type_traits>
#include <cassert>

struct S {
    constexpr S()       : v_(0) {}
    constexpr S(int v)  : v_(v) {}
    constexpr S(const S  &rhs) : v_(rhs.v_) {} // not trivially copyable
    constexpr S(      S &&rhs) : v_(rhs.v_) {} // not trivially moveable
    int v_;
    };


constexpr bool test()
{
    std::optional<S> o1{3};
    std::optional<S> o2 = std::move(o1);  // not constexpr
    return o2.has_value();
}

int main(int, char**)
{
    static_assert(test(), "");
    return 0;
}


fails to compile thus:

$ totclang2a junk.cpp
junk.cpp:14:16: error: constexpr function never produces a constant expression
      [-Winvalid-constexpr]
constexpr bool test()
               ^
junk.cpp:17:27: note: non-constexpr constructor 'optional' cannot be used in a
      constant expression
    std::optional<S> o2 = std::move(o1);  // not constexpr
                          ^
/Sources/LLVM/llvm-project/libcxx/include/optional:689:41: note: declared here
    _LIBCPP_INLINE_VISIBILITY constexpr optional(optional&&) = default;
                                        ^
junk.cpp:24:19: error: static_assert expression is not an integral constant
      expression
    static_assert(test(), "");
                  ^~~~~~
junk.cpp:17:27: note: non-constexpr constructor 'optional' cannot be used in a
      constant expression
    std::optional<S> o2 = std::move(o1);  // not constexpr
                          ^
junk.cpp:24:19: note: in call to 'test()'
    static_assert(test(), "");
                  ^
/Sources/LLVM/llvm-project/libcxx/include/optional:689:41: note: declared here
    _LIBCPP_INLINE_VISIBILITY constexpr optional(optional&&) = default;
                                        ^
2 errors generated.
Comment 3 Marshall Clow (home) 2019-04-24 17:46:15 PDT
Eric pointed out that you can't have a constexpr move-assignment operator for the non-trivial case, because it has to do a placement new (unless the optional that you are constructing from is empty)

That's why my second test fails.
Apparently libstdc++ is happy with this:

constexpr bool test()
{
    std::optional<S> o1; // empty
    std::optional<S> o2 = std::move(o1);
    return o2.has_value();
}

but libc++ is not.
Comment 4 Marshall Clow (home) 2019-04-24 19:16:29 PDT
Test fixed in revision 359162 - now the test is move-constructing a non-empty optional.