[libcxx-commits] [PATCH] D96742: [libcxx] adds concept `std::assignable_from`

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 4 15:25:23 PST 2021


Quuxplusone added a comment.

LGTM mod nits. (Seems like too many tests. The code change itself looks like a correct transliteration of the paper standard.)
aaand whoop, it's already landed :)



================
Comment at: libcxx/test/std/concepts/lang/assignable.compile.pass.cpp:138
+
+  constexpr auto Result = std::assignable_from<T1&, const T2&>;
+  static_assert(std::assignable_from<T1&, T2&> == Result);
----------------
This `auto` means `bool`, right? I'd like to see `bool` here if that's the intention.
(ditto throughout)



================
Comment at: libcxx/test/std/concepts/lang/assignable.compile.pass.cpp:547
+static_assert(!CheckAssignableFromLvaluesAndRvalues<std::vector<int>,
+                                                    std::vector<const int> >());
+static_assert(!CheckAssignableFromLvaluesAndRvalues<
----------------
Here and line 535: is it actually cromulent to create a container with a const (key/element) type? If this is sort of "library IFNDR" then we shouldn't be doing it in tests. But I don't think anything of value would be lost by removing these two lines, anyway, right?


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