Quuxplusone added inline comments.

================
Comment at: clang/test/SemaCXX/implicitly-movable.cpp:14
+
+private:
+  A(const A &);
----------------
aaronpuchert wrote:
> Is this testing what we want it to test? The private functions are just not 
> part of the overload set, right?
> 
> I think you should make them public and `= delete` them.
> The private functions are just not part of the overload set, right?

Short answer "wrong." Overload resolution will make a candidate set consisting 
of //all// the signatures (even the private ones, even the deleted ones), and 
then pick the best match. If the best match is inaccessible (private), or if 
the best match is deleted, then it gives a hard error. So the idea of this test 
is to verify that inside `test_volatile`, Clang doesn't consider `A(volatile 
A&&)` to be the best match.

Making them public and deleted would neither help nor harm, in terms of 
correctness.

However, I agree with your above comment: it does seem like this test should 
actually run the code and verify that the correct overload is getting called. 
Right now it's unclear whether `test_volatile` is calling `A(const volatile 
A&)` as expected; or `A(A&&)`; or nothing at all. (Because copy elision is 
weird, I think we actually expect in this case that it is doing overload 
resolution to require that `A(const volatile A&)` is callable, but then eliding 
the copy anyway.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88295

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to