<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jul 28, 2015 at 12:06 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rtrieu<br>
Date: Tue Jul 28 14:06:16 2015<br>
New Revision: 243463<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D243463-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=NEJNeslqdKDdUJQJJSsQMa_oVx7EUWc23-YBD9LNzuE&s=hY5lQylwJ3APXmhxx2xGpyBWWpJcpM5CW0oNpLtftTI&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=243463&view=rev</a><br>
Log:<br>
Do not give a -Wredundant-move warning when removing the move will result in an<br>
error.<br>
<br>
If the object being moved has a move constructor and a deleted copy constructor,<br>
std::move is required, otherwise Clang will give a deleted constructor error.<br>
<br>
Modified:<br>
    cfe/trunk/lib/Sema/SemaInit.cpp<br>
    cfe/trunk/test/SemaCXX/warn-redundant-move.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaInit.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Sema_SemaInit.cpp-3Frev-3D243463-26r1-3D243462-26r2-3D243463-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=NEJNeslqdKDdUJQJJSsQMa_oVx7EUWc23-YBD9LNzuE&s=9IyYnKppqGWJg9aIAMTEKioGBwi5af2WMytQSjOR3n0&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=243463&r1=243462&r2=243463&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Jul 28 14:06:16 2015<br>
@@ -5983,9 +5983,19 @@ static void CheckMoveOnConstruction(Sema<br>
     if (!VD || !VD->hasLocalStorage())<br>
       return;<br>
<br>
-    if (!VD->getType()->isRecordType())<br>
+    QualType SourceType = VD->getType();<br>
+    if (!SourceType->isRecordType())<br>
       return;<br>
<br>
+    if (!S.Context.hasSameUnqualifiedType(DestType, SourceType)) {<br>
+      if (CXXRecordDecl *RD = SourceType->getAsCXXRecordDecl()) {<br>
+        for (auto* Construct : RD->ctors()) {<br>
+          if (Construct->isCopyConstructor() && Construct->isDeleted())<br>
+            return;<br></blockquote><div><br></div><div>This is not the right change, for a couple of reasons. In particular:</div><div> 1) the constructor that would be selected might not be the copy constructor, so you're not checking the right thing</div><div> 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</div><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+        }<br>
+      }<br>
+    }<br>
+<br>
     // If we're returning a function parameter, copy elision<br>
     // is not possible.<br>
     if (isa<ParmVarDecl>(VD))<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-redundant-move.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_SemaCXX_warn-2Dredundant-2Dmove.cpp-3Frev-3D243463-26r1-3D243462-26r2-3D243463-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=NEJNeslqdKDdUJQJJSsQMa_oVx7EUWc23-YBD9LNzuE&s=zFGU2liwqJZ3ri0C2D2ishETUbCYNaqefwBmjDwpSPA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-redundant-move.cpp?rev=243463&r1=243462&r2=243463&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-redundant-move.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-redundant-move.cpp Tue Jul 28 14:06:16 2015<br>
@@ -1,5 +1,5 @@<br>
 // RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -verify %s<br>
-// RUN: %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s<br>
+// RUN: not %clang_cc1 -fsyntax-only -Wredundant-move -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s<br>
<br>
 // definitions for std::move<br>
 namespace std {<br>
@@ -102,3 +102,39 @@ D test5(D d) {<br>
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:20}:""<br>
   // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:21-[[@LINE-4]]:22}:""<br>
 }<br>
+<br>
+// No more fix-its past here.<br>
+// CHECK-NOT: fix-it<br>
+<br>
+// A deleted copy constructor will prevent moves without std::move<br>
+struct E {<br>
+  E(E &&e);<br>
+  E(const E &e) = delete;<br>
+  // expected-note@-1{{deleted here}}<br>
+};<br>
+<br>
+struct F {<br>
+  F(E);<br>
+  // expected-note@-1{{passing argument to parameter here}}<br>
+};<br>
+<br>
+F test6(E e) {<br>
+  return e;<br>
+  // expected-error@-1{{call to deleted constructor of 'E'}}<br>
+  return std::move(e);<br>
+}<br>
+<br>
+struct G {<br>
+  G(G &&g);<br>
+  // expected-note@-1{{copy constructor is implicitly deleted because 'G' has a user-declared move constructor}}<br>
+};<br>
+<br>
+struct H {<br>
+  H(G);<br>
+  // expected-note@-1{{passing argument to parameter here}}<br>
+};<br>
+<br>
+H test6(G g) {<br>
+  return g;  // expected-error{{call to implicitly-deleted copy constructor of 'G'}}<br>
+  return std::move(g);<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>