r193977 - Issue a diagnostic if an implicitly-defined move assignment operator would move
Richard Smith
richard-llvm at metafoo.co.uk
Sun Nov 3 20:26:14 PST 2013
Author: rsmith
Date: Sun Nov 3 22:26:14 2013
New Revision: 193977
URL: http://llvm.org/viewvc/llvm-project?rev=193977&view=rev
Log:
Issue a diagnostic if an implicitly-defined move assignment operator would move
the same virtual base class multiple times (and the move assignment is used,
and the move assignment for the virtual base is not trivial).
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/special/class.copy/implicit-move-def.cpp
cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=193977&r1=193976&r2=193977&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Nov 3 22:26:14 2013
@@ -6054,6 +6054,13 @@ def err_out_of_line_default_deletes : Er
"defaulting this %select{default constructor|copy constructor|move "
"constructor|copy assignment operator|move assignment operator|destructor}0 "
"would delete it after its first declaration">;
+def warn_vbase_moved_multiple_times : Warning<
+ "defaulted move assignment operator of %0 will move assign virtual base "
+ "class %1 multiple times">, InGroup<DiagGroup<"multiple-move-vbase">>;
+def note_vbase_moved_here : Note<
+ "%select{%1 is a virtual base class of base class %2 declared here|"
+ "virtual base class %1 declared here}0">;
+
def ext_implicit_exception_spec_mismatch : ExtWarn<
"function previously declared with an %select{explicit|implicit}0 exception "
"specification redeclared with an %select{implicit|explicit}0 exception "
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=193977&r1=193976&r2=193977&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sun Nov 3 22:26:14 2013
@@ -9627,6 +9627,94 @@ CXXMethodDecl *Sema::DeclareImplicitMove
return MoveAssignment;
}
+/// Check if we're implicitly defining a move assignment operator for a class
+/// with virtual bases. Such a move assignment might move-assign the virtual
+/// base multiple times.
+static void checkMoveAssignmentForRepeatedMove(Sema &S, CXXRecordDecl *Class,
+ SourceLocation CurrentLocation) {
+ assert(!Class->isDependentContext() && "should not define dependent move");
+
+ // Only a virtual base could get implicitly move-assigned multiple times.
+ // Only a non-trivial move assignment can observe this. We only want to
+ // diagnose if we implicitly define an assignment operator that assigns
+ // two base classes, both of which move-assign the same virtual base.
+ if (Class->getNumVBases() == 0 || Class->hasTrivialMoveAssignment() ||
+ Class->getNumBases() < 2)
+ return;
+
+ llvm::SmallVector<CXXBaseSpecifier *, 16> Worklist;
+ typedef llvm::DenseMap<CXXRecordDecl*, CXXBaseSpecifier*> VBaseMap;
+ VBaseMap VBases;
+
+ for (CXXRecordDecl::base_class_iterator BI = Class->bases_begin(),
+ BE = Class->bases_end();
+ BI != BE; ++BI) {
+ Worklist.push_back(&*BI);
+ while (!Worklist.empty()) {
+ CXXBaseSpecifier *BaseSpec = Worklist.pop_back_val();
+ CXXRecordDecl *Base = BaseSpec->getType()->getAsCXXRecordDecl();
+
+ // If the base has no non-trivial move assignment operators,
+ // we don't care about moves from it.
+ if (!Base->hasNonTrivialMoveAssignment())
+ continue;
+
+ // If there's nothing virtual here, skip it.
+ if (!BaseSpec->isVirtual() && !Base->getNumVBases())
+ continue;
+
+ // If we're not actually going to call a move assignment for this base,
+ // or the selected move assignment is trivial, skip it.
+ Sema::SpecialMemberOverloadResult *SMOR =
+ S.LookupSpecialMember(Base, Sema::CXXMoveAssignment,
+ /*ConstArg*/false, /*VolatileArg*/false,
+ /*RValueThis*/true, /*ConstThis*/false,
+ /*VolatileThis*/false);
+ if (!SMOR->getMethod() || SMOR->getMethod()->isTrivial() ||
+ !SMOR->getMethod()->isMoveAssignmentOperator())
+ continue;
+
+ if (BaseSpec->isVirtual()) {
+ // We're going to move-assign this virtual base, and its move
+ // assignment operator is not trivial. If this can happen for
+ // multiple distinct direct bases of Class, diagnose it. (If it
+ // only happens in one base, we'll diagnose it when synthesizing
+ // that base class's move assignment operator.)
+ CXXBaseSpecifier *&Existing =
+ VBases.insert(std::make_pair(Base->getCanonicalDecl(), BI))
+ .first->second;
+ if (Existing && Existing != BI) {
+ S.Diag(CurrentLocation, diag::warn_vbase_moved_multiple_times)
+ << Class << Base;
+ S.Diag(Existing->getLocStart(), diag::note_vbase_moved_here)
+ << (Base->getCanonicalDecl() ==
+ Existing->getType()->getAsCXXRecordDecl()->getCanonicalDecl())
+ << Base << Existing->getType() << Existing->getSourceRange();
+ S.Diag(BI->getLocStart(), diag::note_vbase_moved_here)
+ << (Base->getCanonicalDecl() ==
+ BI->getType()->getAsCXXRecordDecl()->getCanonicalDecl())
+ << Base << BI->getType() << BaseSpec->getSourceRange();
+
+ // Only diagnose each vbase once.
+ Existing = 0;
+ }
+ } else {
+ // Only walk over bases that have defaulted move assignment operators.
+ // We assume that any user-provided move assignment operator handles
+ // the multiple-moves-of-vbase case itself somehow.
+ if (!SMOR->getMethod()->isDefaulted())
+ continue;
+
+ // We're going to move the base classes of Base. Add them to the list.
+ for (CXXRecordDecl::base_class_iterator BI = Base->bases_begin(),
+ BE = Base->bases_end();
+ BI != BE; ++BI)
+ Worklist.push_back(&*BI);
+ }
+ }
+ }
+}
+
void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
CXXMethodDecl *MoveAssignOperator) {
assert((MoveAssignOperator->isDefaulted() &&
@@ -9656,16 +9744,9 @@ void Sema::DefineImplicitMoveAssignment(
// are assigned, in the order in which they were declared in the class
// definition.
- // FIXME: Issue a warning if our implicit move assignment operator will move
- // from a virtual base more than once. For instance, given:
- //
- // struct A { A &operator=(A&&); };
- // struct B : virtual A {};
- // struct C : virtual A {};
- // struct D : B, C {};
- //
- // If the move assignment operator of D is synthesized, we should warn,
- // because the A vbase will be moved from multiple times.
+ // Issue a warning if our implicit move assignment operator will move
+ // from a virtual base more than once.
+ checkMoveAssignmentForRepeatedMove(*this, ClassDecl, CurrentLocation);
// The statements that form the synthesized function body.
SmallVector<Stmt*, 8> Statements;
@@ -9692,6 +9773,14 @@ void Sema::DefineImplicitMoveAssignment(
bool Invalid = false;
for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(),
E = ClassDecl->bases_end(); Base != E; ++Base) {
+ // C++11 [class.copy]p28:
+ // It is unspecified whether subobjects representing virtual base classes
+ // are assigned more than once by the implicitly-defined copy assignment
+ // operator.
+ // FIXME: Do not assign to a vbase that will be assigned by some other base
+ // class. For a move-assignment, this can result in the vbase being moved
+ // multiple times.
+
// Form the assignment:
// static_cast<Base*>(this)->Base::operator=(static_cast<Base&&>(other));
QualType BaseType = Base->getType().getUnqualifiedType();
Modified: cfe/trunk/test/CXX/special/class.copy/implicit-move-def.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/implicit-move-def.cpp?rev=193977&r1=193976&r2=193977&view=diff
==============================================================================
--- cfe/trunk/test/CXX/special/class.copy/implicit-move-def.cpp (original)
+++ cfe/trunk/test/CXX/special/class.copy/implicit-move-def.cpp Sun Nov 3 22:26:14 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -o - -std=c++11 %s | FileCheck -check-prefix=CHECK-ASSIGN %s
+// FIXME: %clang_cc1 -emit-llvm -o - -std=c++11 %s | FileCheck -check-prefix=CHECK %s
// RUN: %clang_cc1 -emit-llvm -o - -std=c++11 %s | FileCheck -check-prefix=CHECK-ASSIGN %s
// RUN: %clang_cc1 -emit-llvm -o - -std=c++11 %s | FileCheck -check-prefix=CHECK-CTOR %s
Modified: cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp?rev=193977&r1=193976&r2=193977&view=diff
==============================================================================
--- cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp (original)
+++ cfe/trunk/test/CXX/special/class.copy/implicit-move.cpp Sun Nov 3 22:26:14 2013
@@ -243,6 +243,61 @@ namespace DR1402 {
(void)CopyAndMove(cm); // expected-error {{deleted}}
cm = cm; // expected-error {{deleted}}
}
+
+ namespace VbaseMove {
+ struct A {};
+ struct B { B &operator=(B&&); };
+ struct C { C &operator=(const C&); };
+ struct D { B b; };
+
+ template<typename T, unsigned I, bool NonTrivialMove = false>
+ struct E : virtual T {};
+
+ template<typename T, unsigned I>
+ struct E<T, I, true> : virtual T { E &operator=(E&&); };
+
+ template<typename T>
+ struct F :
+ E<T, 0>, // expected-note-re 2{{'[BD]' is a virtual base class of base class 'E<}}
+ E<T, 1> {}; // expected-note-re 2{{'[BD]' is a virtual base class of base class 'E<}}
+
+ template<typename T>
+ struct G : E<T, 0, true>, E<T, 0> {};
+
+ template<typename T>
+ struct H : E<T, 0, true>, E<T, 1, true> {};
+
+ template<typename T>
+ struct I : E<T, 0>, T {};
+
+ template<typename T>
+ struct J :
+ E<T, 0>, // expected-note-re 2{{'[BD]' is a virtual base class of base class 'E<}}
+ virtual T {}; // expected-note-re 2{{virtual base class '[BD]' declared here}}
+
+ template<typename T> void move(T t) { t = static_cast<T&&>(t); }
+ // expected-warning-re at -1 4{{defaulted move assignment operator of .* will move assign virtual base class '[BD]' multiple times}}
+ template void move(F<A>);
+ template void move(F<B>); // expected-note {{in instantiation of}}
+ template void move(F<C>);
+ template void move(F<D>); // expected-note {{in instantiation of}}
+ template void move(G<A>);
+ template void move(G<B>);
+ template void move(G<C>);
+ template void move(G<D>);
+ template void move(H<A>);
+ template void move(H<B>);
+ template void move(H<C>);
+ template void move(H<D>);
+ template void move(I<A>);
+ template void move(I<B>);
+ template void move(I<C>);
+ template void move(I<D>);
+ template void move(J<A>);
+ template void move(J<B>); // expected-note {{in instantiation of}}
+ template void move(J<C>);
+ template void move(J<D>); // expected-note {{in instantiation of}}
+ }
}
namespace PR12625 {
More information about the cfe-commits
mailing list