[libcxx-commits] [PATCH] D96742: [libcxx] adds concept `std::assignable_from`
Christopher Di Bella via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Feb 20 12:08:29 PST 2021
cjdb added a comment.
In D96742#2575681 <https://reviews.llvm.org/D96742#2575681>, @ldionne wrote:
> In D96742#2575636 <https://reviews.llvm.org/D96742#2575636>, @cjdb wrote:
>
>> Agreed. I'll add a few for `vector<int>` and `string`. Any other commonly used types that we should care about (e.g. `map`) or are `vector` and `string` "good enough"?
>
> The more the better, IMO. `map`, `deque`, `unique_ptr`, `variant` (?), just a few of the common ones. I doubt it'll actually help us find any issues with the concepts themselves, but it could potentially find us issues with those types!
I think it's a great idea, and one I'm happy to take on; but if we do it right now, any latent bugs in other types that we uncover will become blockers for getting the rest of the concepts merged into libc++ (see my recent email <https://lists.llvm.org/pipermail/libcxx-dev/2021-February/001081.html> for a potentially large blocker). I propose making a separate patch for each concept where we test all the frequently used standard types. Those patches can be indefinitely blocked, which isn't great, but at least it doesn't prevent libc++ from being C++20 feature-complete in a reasonable amount of time.
To reiterate: I'm not shrugging off this work, but I'd like to defer it to after `__cpp_lib_concepts` is defined. If someone is motivated to pick it up before then, feel free to do so: I can confirm at least one major issue will fall out of `std::ranges::swap`.
================
Comment at: libcxx/test/std/concepts/lang/assignable.compile.pass.cpp:21
+constexpr bool ModelsAssignableFrom() {
+ static_assert(!std::assignable_from<T1, T2>);
+ static_assert(!std::assignable_from<T1, const T2>);
----------------
ldionne wrote:
> Should we use the trick that Casey mentioned in another patch where we basically test `static_assert(std::assignable_from<T1, const T2> == std::assignable_from<T1, T2>);`?
>
> That way, you could use something like this below: `static_assert(!ModelsAssignableFrom<int*&, const int*>());`
(Note: this is from D88131).
I thought I was emulating Casey's stuff, but seems it can be improved :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96742/new/
https://reviews.llvm.org/D96742
More information about the libcxx-commits
mailing list