[libcxx-commits] [PATCH] D97443: [libcxx] adds concept std::copyable

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/concepts/object/copyable.h:53
+  operator=(volatile_copy_assignment const&) volatile;
+};
+
----------------
cjdb wrote:
> Quuxplusone wrote:
> > This naming convention implies more structure than you really have here. I think `struct X` might be a better name for this particular grab-bag of special member signatures.
> > Alternatively, it might arguably be important to test some type that //is copyable// but volatile-qualified. In that case, I think the minimal set of SMFs that makes the type //actually// copyable would be
> > https://godbolt.org/z/8affTh
> > ```
> > struct volatile_copy_assignment {
> >   volatile_copy_assignment(volatile_copy_assignment const volatile&);
> >   volatile_copy_assignment(volatile_copy_assignment const volatile&&);
> > 
> >   volatile_copy_assignment volatile& operator=(volatile_copy_assignment const volatile&) volatile;
> >   volatile_copy_assignment volatile& operator=(volatile_copy_assignment const volatile&&) volatile;
> > };
> > ```
> > (It's trickier than it sounds at first glance, because even though an `X&&` argument binds to a `const X&` parameter, yet a `volatile X&&` argument does //not// bind to a `const volatile X&` parameter.)
> Isn't that what `cv_copy_assignment` does below?
No, `cv_copy_assignment` looks like it's trying to achieve `std::copyable<const volatile cv_copy_assignment>`. It does //not// achieve `std::copyable<volatile cv_copy_assignment>`, because the return type of `operator=` is wrong for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97443