This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Extension: Make `move` and `forward` constexpr in C++11.
ClosedPublic

Authored by EricWF on Sep 15 2016, 3:14 PM.

Details

Summary

std::move and std::forward were not marked constexpr in C++11. This can be very damaging because it makes otherwise constant expressions non-constant. For example:

#include <utility>
template <class T>
struct Foo {
  constexpr Foo(T&& tx) :  t(std::move(tx)) {}
  T t;
};
[[clang::require_constant_initialization]] Foo<int> f(42); // Foo should be constant initialized but C++11 move is not constexpr. As a result `f` is an unsafe global.

This patch applies constexpr to move and forward as an extension in C++11. Normally the library is not allowed to add constexpr because it may be observable to the user. In particular adding constexpr may cause valid code to stop compiling. However these problems only happen in more complex situations, like making __invoke(...) constexpr. forward and move are simply enough that applying constexpr is safe.

Note that libstdc++ has offered this extension since at least 4.8.1.

Most of the changes in this patch are simply test cleanups or additions. The main changes in the tests are:

  • Fold all forward_N.fail.cpp tests into a single forward.fail.cpp test using -verify.
  • Delete most move_only_N.fail.cpp tests because they weren't actually testing anything.
  • Fold move_copy.pass.cpp and move_only.pass.cpp into a single move.pass.cpp test.
  • Add return type and noexcept tests for forward and move.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 71564.Sep 15 2016, 3:14 PM
EricWF retitled this revision from to [libc++] Extension: Make `move` and `forward` constexpr in C++11..
EricWF updated this object.
EricWF added reviewers: mclow.lists, rsmith.
EricWF added subscribers: loladiro, K-ballo.

I like this!

EricWF updated this revision to Diff 71568.Sep 15 2016, 3:22 PM

Use "an lvalue" and "an rvalue" instead of "a lvalue" and "a rvalue" in the static assert message.

rsmith edited edge metadata.Sep 15 2016, 5:58 PM

Do we expect [constexpr.functions] / LWG issue 2013 to reverse its direction any time soon? (Are you perhaps hoping to use implementation consensus here to encourage the direction to be reversed?)

Do we expect [constexpr.functions] / LWG issue 2013 to reverse its direction any time soon? (Are you perhaps hoping to use implementation consensus here to encourage the direction to be reversed?)

I don't expect it to be reversed, nor am I in support of reversing it until Clang and GCC agree on eager instantiation in unevaluated contexts. This is meant only as a targeted fix, not to set precedent.

Do we expect [constexpr.functions] / LWG issue 2013 to reverse its direction any time soon? (Are you perhaps hoping to use implementation consensus here to encourage the direction to be reversed?)

I don't expect it to be reversed, nor am I in support of reversing it until Clang and GCC agree on eager instantiation in unevaluated contexts. This is meant only as a targeted fix, not to set precedent.

To be clear I understand this extension is explicitly forbidden. However I think this patch is more of a "defect resolution" than an extension. I'm confident that the definitions of move/forward are simple enough that applying constexpr shouldn't lead to surprises (Unlike when I applied constexpr to __invoke(...)), unlike the current behavior which is quite surprising since it can silently change the meaning of code between C++11 and C++14 (e.g. static initialization semantics).

As an additional benefit libc++ requires a constexpr move/forward internally and I would hate to start using __move and __forward but that's a "me" problem.

mclow.lists edited edge metadata.EditedSep 15 2016, 7:23 PM

My concern here is that (as Eric says) WG21 has decreed that implementations are not allowed to add constexpr to things that are not constexpr in the standard (unlike noexcept, where they are). I don't really agree with this prohibition, but it exists.

@EricWF's point about libstdc++ already doing this is telling, though. (they also have added constexpr to bits of std::pair - but I don't remember where.)

Also, these calls *are* constexpr in c++14 and later; he's only proposing to make them constexpr in C++11 as well

I'm coming around to the view that making move and forward constexpr in C++11 is the way to go.

EricWF accepted this revision.Sep 26 2016, 2:03 PM
EricWF added a reviewer: EricWF.

Marshall and I agree this is the correct direction.

This revision is now accepted and ready to land.Sep 26 2016, 2:03 PM
EricWF closed this revision.Sep 26 2016, 2:03 PM
test/std/utilities/utility/forward/move_only3.fail.cpp