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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
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<
----------------
cjdb wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > 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?
> > Hmm, I had missed that indeed but `std::vector<T const>` isn't allowed because `T const` isn't movable. I think we should remove it. @cjdb did you do this in other tests too? If so, we should fix them too.
> I'm a bit confused as to why we say that `std::vector<T const>` isn't allowed. It shouldn't be because `T const` isn't movable: that'd mean `std::vector<std::mutex>` is also disallowed (and as frustrating as it is to use, that type works "fine").
FWIW: I've looked at similar issues before (sadly don't remember exactly which containers — probably set or map) where ultimately the pragmatic problem was that two member functions ended up with the same signature — e.g. if you had `void foo(iterator)` and `void foo(const_iterator)`, and both of those types were `const T*` (or `iteratorImpl<const T>` or something), then boom, collision. I don't know if formal wording exists to make it ill-formed. I //do// agree with you that `vector<[non-const] mutex>` is supported in practice; I'm //only// objecting to explicitly cv-qualified element types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96742/new/

https://reviews.llvm.org/D96742