On Tue, Jul 28, 2015 at 12:06 PM, Richard Trieu <[email protected]> wrote:
> Author: rtrieu > Date: Tue Jul 28 14:06:16 2015 > New Revision: 243463 > > URL: http://llvm.org/viewvc/llvm-project?rev=243463&view=rev > Log: > Do not give a -Wredundant-move warning when removing the move will result > in an > error. > > If the object being moved has a move constructor and a deleted copy > constructor, > std::move is required, otherwise Clang will give a deleted constructor > error. > > Modified: > cfe/trunk/lib/Sema/SemaInit.cpp > cfe/trunk/test/SemaCXX/warn-redundant-move.cpp > > Modified: cfe/trunk/lib/Sema/SemaInit.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=243463&r1=243462&r2=243463&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaInit.cpp (original) > +++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Jul 28 14:06:16 2015 > @@ -5983,9 +5983,19 @@ static void CheckMoveOnConstruction(Sema > if (!VD || !VD->hasLocalStorage()) > return; > > - if (!VD->getType()->isRecordType()) > + QualType SourceType = VD->getType(); > + if (!SourceType->isRecordType()) > return; > > + if (!S.Context.hasSameUnqualifiedType(DestType, SourceType)) { > + if (CXXRecordDecl *RD = SourceType->getAsCXXRecordDecl()) { > + for (auto* Construct : RD->ctors()) { > + if (Construct->isCopyConstructor() && Construct->isDeleted()) > + return; > This is not the right change, for a couple of reasons. In particular: 1) the constructor that would be selected might not be the copy constructor, so you're not checking the right thing 2) the purpose of the warning is to warn on cases where you'd get an implicit move even without the std::move call, and you seem to now be looking for cases where the call to std::move would result in a copy Until we have an implementation of DR1579, the best thing to do is probably just to disable/remove the -Wredundant-move warning. As far as I recall, its only purpose was to warn on the cases where DR1579 applies, and there are no such cases since we don't implement DR1579. > + } > + } > + } > + > // If we're returning a function parameter, copy elision > // is not possible. > if (isa<ParmVarDecl>(VD)) > > Modified: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-redundant-move.cpp?rev=243463&r1=243462&r2=243463&view=diff > > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (original) > +++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Tue Jul 28 14:06:16 2015 > @@ -1,5 +1,5 @@ > // RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -verify %s > -// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 > -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s > +// RUN: not %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 > -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s > > // definitions for std::move > namespace std { > @@ -102,3 +102,39 @@ D test5(D d) { > // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:"" > // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:"" > } > + > +// No more fix-its past here. > +// CHECK-NOT: fix-it > + > +// A deleted copy constructor will prevent moves without std::move > +struct E { > + E(E &&e); > + E(const E &e) = delete; > + // expected-note@-1{{deleted here}} > +}; > + > +struct F { > + F(E); > + // expected-note@-1{{passing argument to parameter here}} > +}; > + > +F test6(E e) { > + return e; > + // expected-error@-1{{call to deleted constructor of 'E'}} > + return std::move(e); > +} > + > +struct G { > + G(G &&g); > + // expected-note@-1{{copy constructor is implicitly deleted because > 'G' has a user-declared move constructor}} > +}; > + > +struct H { > + H(G); > + // expected-note@-1{{passing argument to parameter here}} > +}; > + > +H test6(G g) { > + return g; // expected-error{{call to implicitly-deleted copy > constructor of 'G'}} > + return std::move(g); > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
