This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] adds concept `std::copy_constructible`
ClosedPublic

Authored by cjdb on Feb 7 2021, 7:04 PM.

Details

Summary

Implements parts of:

  • P0898R3 Standard Library Concepts
  • P1754 Rename concepts to standard_case for C++20, while we still can

Depends on D96230

Diff Detail

Event Timeline

cjdb requested review of this revision.Feb 7 2021, 7:04 PM
cjdb created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2021, 7:04 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Also look at the remarks in D96230 that are applicable here.

libcxx/test/std/concepts/lang/copyconstructible.pass.cpp
12 ↗(On Diff #322016)

This seems to be a copy-paste. copy_constructible has only one template argument.

libcxx/test/std/concepts/lang/moveconstructible.h
1 ↗(On Diff #322016)

Misses the header block. Would it make sense to implement this header already in D96230?

cjdb updated this revision to Diff 322284.Feb 8 2021, 9:51 PM
cjdb marked an inline comment as done.

applies fixes wrt comments

cjdb updated this revision to Diff 322289.Feb 8 2021, 9:55 PM

fixes rebase issue

cjdb added a comment.Feb 8 2021, 9:56 PM

Also look at the remarks in D96230 that are applicable here.

Same comments apply.

Mordante accepted this revision.Feb 9 2021, 9:49 AM

LGTM!

cjdb updated this revision to Diff 322488.Feb 9 2021, 1:35 PM

adds #include "test_macros.h"

ldionne accepted this revision.Feb 9 2021, 2:55 PM
ldionne added inline comments.
libcxx/test/std/concepts/lang/copyconstructible.compile.pass.cpp
22

I'm not sure I follow that comment, but it's non-blocking.

This revision is now accepted and ready to land.Feb 9 2021, 2:55 PM
cjdb updated this revision to Diff 322578.Feb 9 2021, 8:16 PM

rebases to make sure CI is happy

cjdb added inline comments.Feb 9 2021, 8:25 PM
libcxx/test/std/concepts/lang/copyconstructible.compile.pass.cpp
22

Does s/common/shared/ make it a little clearer?

ldionne accepted this revision.Feb 10 2021, 8:46 AM
ldionne added inline comments.
libcxx/test/std/concepts/lang/copyconstructible.compile.pass.cpp
22

Ah, I looked at the rev for move_constructible and I understand better. Feel free to change the comment or not.

If we have to duplicate this list of types in a bunch of places, we should consider storing it in a single place with a way to reuse it across tests.

This revision was automatically updated to reflect the committed changes.
cjdb added inline comments.Feb 10 2021, 10:02 AM
libcxx/test/std/concepts/lang/copyconstructible.compile.pass.cpp
22

I'd like to do that, but it's not feasible without macros (concepts can't be template parameters). Your call here, but I'm... not in favour of that.