This is an archive of the discontinued LLVM Phabricator instance.

Implement `std::launder`
ClosedPublic

Authored by mclow.lists on Nov 16 2017, 11:50 AM.

Details

Reviewers
EricWF
rsmith
Summary

std::launder was introduced into c++17 as a compiler optimization barrier. It's something that the compiler 'knows about', and affects codegen.

See https://wg21.link/p0137r1 for more.

Diff Detail

Event Timeline

mclow.lists created this revision.Nov 16 2017, 11:50 AM
Quuxplusone added inline comments.
test/std/language.support/support.dynamic/ptr.launder/launder.fail.cpp
26 ↗(On Diff #123226)

Would one also expect a decent static analyzer to warn that you are laundering a null pointer? (and should we care?)

efriedma added inline comments.
include/new
174

How is the compiler supposed to know that "std::__1::launder()" has special semantics?

efriedma added inline comments.Nov 16 2017, 12:17 PM
include/new
174

Oh, wait, is this actually not in the __1 namespace? Sort of hard to tell because the patch wasn't posted with enough context.

It isn't exactly great to special-case functions named "std::launder"... but wouldn't be the first name in the std namespace which has special compiler semantics.

At least for GCC, it should use __builtin_launder.

Also needs to implement "The program is ill-formed if T is a function type or cv void."

At least for GCC, it should use __builtin_launder.

I presume we'll need to add something similar for Clang as well.

Also needs to implement "The program is ill-formed if T is a function type or cv void."

rsmith edited edge metadata.Nov 16 2017, 2:31 PM

At least for GCC, it should use __builtin_launder.

I presume we'll need to add something similar for Clang as well.

Yes, we agreed to use the same builtin name for this in Clang and GCC quite a while back, but haven't actually got around to implementing it in Clang yet. You should use __has_builtin to detect the existence of this builtin in Clang. In GCC, I believe this is available iff __GNUC__ is 7 or greater.

mclow.lists marked an inline comment as done.Nov 17 2017, 2:31 PM
mclow.lists added inline comments.
include/new
174

I'm about to move it into the _1 namespace. Since it's calling a compiler intrinsic, it doesn't need to be in a special place.

mclow.lists updated this revision to Diff 123422.EditedNov 17 2017, 2:32 PM
mclow.lists marked an inline comment as done.

Move the launder function into the main libc++ namespace.
Call __builtin_launder when available.
Check to see when it's available (for gcc and clang)
Add failing tests.

EricWF edited edge metadata.Nov 18 2017, 12:30 PM

I think we may want a __launder function that we can use internally in all dialects.

include/__config
458

These macros should take the negative _LIBCPP_HAS_NO_BUILTIN_LAUNDER form to be consistent.

include/new
260

Typically diagnostics don't start with capitals.

Small nit on the use of a contraction too.

261

Since this is C++17 only, we can use the trait_t and trait_v versions to be less verbose.

I've made an attempt to add __builtin_launder to clang in D40218.

Made an internal function __launder which is not c++17 specific.
Fixed some wording for the standard asserts.

mclow.lists marked 3 inline comments as done.Nov 20 2017, 1:13 PM
tcanens added inline comments.Nov 20 2017, 1:22 PM
include/new
260

Technically, the attempt is to launder pointers to cv void/functions. :)

Also, since __launder is non-C++17-specific, I guess it can't use _t and _v after all... - and I suppose it'll also need to parens-protect the is_same check for the fallback static_assert macro?

mclow.lists added inline comments.Nov 20 2017, 2:48 PM
include/new
260

Oh, *bother* (and yes, you're correct)

Un-c++17'ed the internal function __launder

mclow.lists marked 2 inline comments as done.Nov 20 2017, 2:51 PM
EricWF added inline comments.Nov 21 2017, 7:22 PM
include/new
274

The call should probably be qualified to _VSTD::__launder.

test/std/language.support/support.dynamic/ptr.launder/launder.nodiscard.fail.cpp
17

Why is this test unsupported with older compilers? The version of std::launder in this patch should work regardless of compiler support for __builtin_launder.

Although, it's possible the compiler requirements were intended to avoid older clang versions without [[nodiscard]]. However, I don't think clang-3.8 supports C++2a, so the condition seems redundant.

test/std/language.support/support.dynamic/ptr.launder/launder.types.fail.cpp
29

Alternatively you could de-duplicate the four identical checks into the single check:

// expected-error-re@new:* 4 {{static_assert failed{{.*}} "can't launder cv-void"}}
mclow.lists added inline comments.Nov 21 2017, 7:26 PM
test/std/language.support/support.dynamic/ptr.launder/launder.nodiscard.fail.cpp
17

We're testing the [[nodiscard]] bit here, not the launder bit. And that didn't come in until clang 3.9

EricWF added inline comments.Nov 21 2017, 7:32 PM
test/std/language.support/support.dynamic/ptr.launder/launder.nodiscard.fail.cpp
17

Perhaps we should fall back to __attribute__((warn_unused_result)) when we don't have [[nodiscard]]? All supported Clang versions should provide that, as well as GCC 4.9 and newer.

_VSTD:: qualify the call to __launder.
De-dup error messages in test.

EricWF accepted this revision.Nov 22 2017, 11:30 AM
This revision is now accepted and ready to land.Nov 22 2017, 11:30 AM
mclow.lists closed this revision.Nov 22 2017, 11:50 AM

landed as revision 318864