Author: Arthur O'Dwyer Date: 2021-03-23T14:12:06-04:00 New Revision: 5f1de9cab1ce2fbb1e45239d3e0e8d4970d2124e
URL: https://github.com/llvm/llvm-project/commit/5f1de9cab1ce2fbb1e45239d3e0e8d4970d2124e DIFF: https://github.com/llvm/llvm-project/commit/5f1de9cab1ce2fbb1e45239d3e0e8d4970d2124e.diff LOG: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type. Review D88220 turns out to have some pretty severe bugs, but I *think* this patch fixes them. Paper 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 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. Differential Revision: https://reviews.llvm.org/D98971 Added: Modified: clang/lib/Sema/SemaStmt.cpp clang/test/CXX/class/class.init/class.copy.elision/p3.cpp Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 4a275e6bd2a1..ceba83bcd814 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3092,24 +3092,30 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, 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 (VDType->isObjectType()) { + // C++17 [class.copy.elision]p3: + // ...non-volatile automatic object... + if (VDType.isVolatileQualified()) + return false; + } else if (VDType->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; + QualType VDReferencedType = VDType.getNonReferenceType(); + if (VDReferencedType.isVolatileQualified() || !VDReferencedType->isObjectType()) + return false; + } else { return false; + } if (CESK & CES_AllowDifferentTypes) return true; // Variables with higher required alignment than their type's ABI // alignment cannot use NRVO. - if (!VD->getType()->isDependentType() && VD->hasAttr<AlignedAttr>() && - Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VD->getType())) + if (!VDType->isDependentType() && VD->hasAttr<AlignedAttr>() && + Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType)) return false; return true; diff --git a/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp index 29d818602537..e4056221b4f3 100644 --- a/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp +++ b/clang/test/CXX/class/class.init/class.copy.elision/p3.cpp @@ -292,3 +292,108 @@ NeedValue test_4_3() { 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits