This is an archive of the discontinued LLVM Phabricator instance.

[libc++][test] Use = delete over DELETE_FUNCTION. NFC.
ClosedPublic

Authored by jloser on Oct 5 2021, 6:40 AM.

Details

Summary

Some tests repeat the definition of DELETE_FUNCTION macro locally.
However, it's not even requred to guard against in the C++03 case since
Clang supports = delete; in C++03 mode. A warning is issued but
libc++ tests run with -Wno-c++11-extensions, so this isn't an issue.
Since we don't support other compilers in C++03 mode, = delete; is
always available for use. As such, inline all calls of DELETE_FUNCTION
to use = delete;.

Diff Detail

Event Timeline

jloser requested review of this revision.Oct 5 2021, 6:40 AM
jloser created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 5 2021, 6:40 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
jloser updated this revision to Diff 377204.Oct 5 2021, 6:55 AM
jloser retitled this revision from [libc++[test] Move DELETE_FUNCTION into test_macros.h. NFC. to [libc++][test] Move DELETE_FUNCTION into test_macros.h. NFC..
jloser edited the summary of this revision. (Show Details)

Fix commit message

jloser updated this revision to Diff 377206.Oct 5 2021, 6:56 AM

Use DELETE_FUNCTION in test_allocator.h

jloser updated this revision to Diff 377224.Oct 5 2021, 7:03 AM

Remove extra private section in limited_allocator since we use DELETE_FUNCTION now.

jloser updated this revision to Diff 377227.Oct 5 2021, 7:05 AM

[NFC] More cleanup in limited_allocator

LGTM, but, have you tried just inlining = delete; in all these places? I believe Clang supports it in C++03 mode, and we don't care about other compilers in C++03 mode.

jloser updated this revision to Diff 377237.Oct 5 2021, 7:47 AM
jloser retitled this revision from [libc++][test] Move DELETE_FUNCTION into test_macros.h. NFC. to [libc++][test] Use = delete over DELETE_FUNCTION. NFC..
jloser edited the summary of this revision. (Show Details)

Inline = delete; for all usages of DELETE_FUNCTION

jloser added a comment.Oct 5 2021, 7:48 AM

LGTM, but, have you tried just inlining = delete; in all these places? I believe Clang supports it in C++03 mode, and we don't care about other compilers in C++03 mode.

Just tried inlining = delete; everywhere. We'll see if BuildKite is happy! It seems you're right that Clang supports it in C++03 mode. A warning is issued, but the libc++ tests run with -Wno-c++11-extensions, so we should be OK. Small example at https://godbolt.org/z/KvYKMqWfz

ldionne accepted this revision.Oct 5 2021, 8:45 AM

LGTM if CI is happy.

This revision is now accepted and ready to land.Oct 5 2021, 8:45 AM