Quuxplusone created this revision.
Quuxplusone added reviewers: rsmith, nullptr.cpp, aaronpuchert, mizvekov,
erik.pilkington, sberg, davidstone, david_stone.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.
Review D88220 <https://reviews.llvm.org/D88220> turns out to have some pretty
severe bugs, but I *think* this patch fixes them.
Paper P1825 <https://reviews.llvm.org/P1825> is supposed to enable implicit
move from "non-volatile objects
and rvalue references to non-volatile object types." Instead, what was committed
seems to have enabled implicit move from "non-volatile things of all kinds,
except that if they're rvalue references then they must also refer to
non-volatile
things." In other words, D88220 <https://reviews.llvm.org/D88220> accidentally
enabled implicit move from
lvalue object references (super yikes!) and also from non-object references
(such as references to functions).
These two cases are now fixed and regression-tested.
(For better and worse, D88220 <https://reviews.llvm.org/D88220> was present in
trunk for more than a month before anyone noticed the (very major!) bug — my
thanks to @sberg for reporting it! — so we may be able to take our time fixing
it, but also it may be another month before someone spots the //next// bug. I
have no strong opinion on the process here.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D98971
Files:
clang/lib/Sema/SemaStmt.cpp
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===================================================================
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -292,3 +292,108 @@
return b; // cxx20-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
}
} // namespace test_ctor_param_rvalue_ref
+
+namespace test_lvalue_ref_is_not_moved_from {
+
+struct Target {};
+ // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
+ // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable}}
+ // cxx11_14_17-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+ // cxx11_14_17-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct CopyOnly {
+ CopyOnly(CopyOnly&&) = delete; // cxx20-note {{has been explicitly marked deleted here}}
+ CopyOnly(CopyOnly&);
+ operator Target() && = delete; // cxx20-note {{has been explicitly marked deleted here}}
+ operator Target() &;
+};
+
+struct MoveOnly {
+ MoveOnly(MoveOnly&&); // expected-note {{copy constructor is implicitly deleted because}}
+ // cxx11_14_17-note@-1 {{copy constructor is implicitly deleted because}}
+ operator Target() &&; // expected-note {{candidate function not viable}}
+ // cxx11_14_17-note@-1 {{candidate function not viable}}
+};
+
+extern CopyOnly copyonly;
+extern MoveOnly moveonly;
+
+CopyOnly t1() {
+ CopyOnly& r = copyonly;
+ return r;
+}
+
+CopyOnly t2() {
+ CopyOnly&& r = static_cast<CopyOnly&&>(copyonly);
+ return r; // cxx20-error {{call to deleted constructor}}
+}
+
+MoveOnly t3() {
+ MoveOnly& r = moveonly;
+ return r; // expected-error {{call to implicitly-deleted copy constructor}}
+}
+
+MoveOnly t4() {
+ MoveOnly&& r = static_cast<MoveOnly&&>(moveonly);
+ return r; // cxx11_14_17-error {{call to implicitly-deleted copy constructor}}
+}
+
+Target t5() {
+ CopyOnly& r = copyonly;
+ return r;
+}
+
+Target t6() {
+ CopyOnly&& r = static_cast<CopyOnly&&>(copyonly);
+ return r; // cxx20-error {{invokes a deleted function}}
+}
+
+Target t7() {
+ MoveOnly& r = moveonly;
+ return r; // expected-error {{no viable conversion}}
+}
+
+Target t8() {
+ MoveOnly&& r = static_cast<MoveOnly&&>(moveonly);
+ return r; // cxx11_14_17-error {{no viable conversion}}
+}
+
+} // namespace test_lvalue_ref_is_not_moved_from
+
+namespace test_rvalue_ref_to_nonobject {
+
+struct CopyOnly {};
+struct MoveOnly {};
+
+struct Target {
+ Target(CopyOnly (&)());
+ Target(CopyOnly (&&)()) = delete;
+ Target(MoveOnly (&)()) = delete; // expected-note {{has been explicitly marked deleted here}}
+ // expected-note@-1 {{has been explicitly marked deleted here}}
+ Target(MoveOnly (&&)());
+};
+
+CopyOnly make_copyonly();
+MoveOnly make_moveonly();
+
+Target t1() {
+ CopyOnly (&r)() = make_copyonly;
+ return r;
+}
+
+Target t2() {
+ CopyOnly (&&r)() = static_cast<CopyOnly(&&)()>(make_copyonly);
+ return r; // OK in all modes; not subject to implicit move
+}
+
+Target t3() {
+ MoveOnly (&r)() = make_moveonly;
+ return r; // expected-error {{invokes a deleted function}}
+}
+
+Target t4() {
+ MoveOnly (&&r)() = static_cast<MoveOnly(&&)()>(make_moveonly);
+ return r; // expected-error {{invokes a deleted function}}
+}
+
+} // namespace test_rvalue_ref_to_nonobject
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3092,16 +3092,23 @@
if (VD->hasAttr<BlocksAttr>())
return false;
- // ...non-volatile...
- if (VD->getType().isVolatileQualified())
- return false;
-
- // C++20 [class.copy.elision]p3:
- // ...rvalue reference to a non-volatile...
- if (VD->getType()->isRValueReferenceType() &&
- (!(CESK & CES_AllowRValueReferenceType) ||
- VD->getType().getNonReferenceType().isVolatileQualified()))
+ if (VD->getType()->isObjectType()) {
+ // C++17 [class.copy.elision]p3:
+ // ...non-volatile automatic object...
+ if (VD->getType().isVolatileQualified())
+ return false;
+ } else if (VD->getType()->isRValueReferenceType()) {
+ // C++20 [class.copy.elision]p3:
+ // ...either a non-volatile object or an rvalue reference to a non-volatile object type...
+ if (!(CESK & CES_AllowRValueReferenceType))
+ return false;
+ if (!VD->getType().getNonReferenceType()->isObjectType())
+ return false;
+ if (VD->getType().getNonReferenceType().isVolatileQualified())
+ return false;
+ } else {
return false;
+ }
if (CESK & CES_AllowDifferentTypes)
return true;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
