<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 12, 2014 at 2:05 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 Aug 12 16:05:04 2014<br>
New Revision: 215471<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215471&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215471&view=rev</a><br>
Log:<br>
Improve -Wuninitialized to catch const classes being used in their own copy<br>
constructors.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
cfe/trunk/test/SemaCXX/uninitialized.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=215471&r1=215470&r2=215471&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=215471&r1=215470&r2=215471&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Aug 12 16:05:04 2014<br>
@@ -8276,6 +8276,15 @@ namespace {<br>
<br>
void VisitObjCMessageExpr(ObjCMessageExpr *E) { return; }<br>
<br>
+ void VisitCXXConstructExpr(CXXConstructExpr *E) {<br>
+ if (E->getConstructor()->isCopyConstructor()) {<br></blockquote><div><br></div><div>Should this also apply to move constructors? I'd think it'd help in cases like:</div><div><br></div><div> void f(string foo_) {</div>
<div> string foo(std::move(foo)); // oops</div><div><br></div><div>... or </div><div><br></div><div> S::S(T x) : x_(x_) {} // oops</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->getArg(0))) {<br>
+ HandleDeclRefExpr(DRE);<br>
+ }<br>
+ }<br>
+ Inherited::VisitCXXConstructExpr(E);<br>
+ }<br>
+<br>
void HandleDeclRefExpr(DeclRefExpr *DRE) {<br>
Decl* ReferenceDecl = DRE->getDecl();<br>
if (OrigDecl != ReferenceDecl) return;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=215471&r1=215470&r2=215471&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=215471&r1=215470&r2=215471&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Aug 12 16:05:04 2014<br>
@@ -2314,12 +2314,18 @@ namespace {<br>
}<br>
<br>
void VisitCXXConstructExpr(CXXConstructExpr *E) {<br>
- if (E->getConstructor()->isCopyConstructor())<br>
- if (ImplicitCastExpr* ICE = dyn_cast<ImplicitCastExpr>(E->getArg(0)))<br>
- if (ICE->getCastKind() == CK_NoOp)<br>
- if (MemberExpr *ME = dyn_cast<MemberExpr>(ICE->getSubExpr()))<br>
- HandleMemberExpr(ME, false /*CheckReferenceOnly*/);<br>
-<br>
+ if (E->getConstructor()->isCopyConstructor()) {<br>
+ Expr *ArgExpr = E->getArg(0);<br>
+ if (ImplicitCastExpr* ICE = dyn_cast<ImplicitCastExpr>(ArgExpr)) {<br>
+ if (ICE->getCastKind() == CK_NoOp) {<br>
+ ArgExpr = ICE->getSubExpr();<br>
+ }<br>
+ }<br></blockquote><div><br></div><div>We should probably IgnoreParenImpCasts() or similar here. Trying to exactly match the shape of implicit AST nodes is bound to be bug-prone.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(ArgExpr)) {<br>
+ HandleMemberExpr(ME, false /*CheckReferenceOnly*/);<br>
+ }<br>
+ }<br>
Inherited::VisitCXXConstructExpr(E);<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=215471&r1=215470&r2=215471&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=215471&r1=215470&r2=215471&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/uninitialized.cpp Tue Aug 12 16:05:04 2014<br>
@@ -127,6 +127,9 @@ void setupA(bool x) {<br>
A *a26 = new A(a26->get()); // expected-warning {{variable 'a26' is uninitialized when used within its own initialization}}<br>
A *a27 = new A(a27->get2()); // expected-warning {{variable 'a27' is uninitialized when used within its own initialization}}<br>
A *a28 = new A(a28->num); // expected-warning {{variable 'a28' is uninitialized when used within its own initialization}}<br>
+<br>
+ const A a29(a29); // expected-warning {{variable 'a29' is uninitialized when used within its own initialization}}<br>
+ const A a30 = a30; // expected-warning {{variable 'a30' is uninitialized when used within its own initialization}}<br>
}<br>
<br>
bool x;<br>
@@ -163,6 +166,9 @@ A *a26 = new A(a26->get()); // expect<br>
A *a27 = new A(a27->get2()); // expected-warning {{variable 'a27' is uninitialized when used within its own initialization}}<br>
A *a28 = new A(a28->num); // expected-warning {{variable 'a28' is uninitialized when used within its own initialization}}<br>
<br>
+const A a29(a29); // expected-warning {{variable 'a29' is uninitialized when used within its own initialization}}<br>
+const A a30 = a30; // expected-warning {{variable 'a30' is uninitialized when used within its own initialization}}<br>
+<br>
struct B {<br>
// POD struct.<br>
int x;<br>
@@ -209,6 +215,10 @@ void setupB() {<br>
<br>
B b17 = { b17.x = 5, b17.y = 0 };<br>
B b18 = { b18.x + 1, b18.y }; // expected-warning 2{{variable 'b18' is uninitialized when used within its own initialization}}<br>
+<br>
+ const B b19 = b19; // expected-warning {{variable 'b19' is uninitialized when used within its own initialization}}<br>
+ const B b20(b20); // expected-warning {{variable 'b20' is uninitialized when used within its own initialization}}<br>
+<br>
}<br>
<br>
B b1;<br>
@@ -234,6 +244,8 @@ B* b16 = getPtrB(b16->y); // expected-w<br>
B b17 = { b17.x = 5, b17.y = 0 };<br>
B b18 = { b18.x + 1, b18.y }; // expected-warning 2{{variable 'b18' is uninitialized when used within its own initialization}}<br>
<br>
+const B b19 = b19; // expected-warning {{variable 'b19' is uninitialized when used within its own initialization}}<br>
+const B b20(b20); // expected-warning {{variable 'b20' is uninitialized when used within its own initialization}}<br>
<br>
// Also test similar constructs in a field's initializer.<br>
struct S {<br>
@@ -385,6 +397,11 @@ namespace {<br>
G(char (*)[7]) : f3(f3->*f_ptr) {} // expected-warning {{field 'f3' is uninitialized when used here}}<br>
G(char (*)[8]) : f3(new F(f3->*ptr)) {} // expected-warning {{field 'f3' is uninitialized when used here}}<br>
};<br>
+<br>
+ struct H {<br>
+ H() : a(a) {} // expected-warning {{field 'a' is uninitialized when used here}}<br>
+ const A a;<br>
+ };<br>
}<br>
<br>
namespace statics {<br>
@@ -555,7 +572,7 @@ namespace record_fields {<br>
B(char (*)[9]) : a(normal(a)) {} // expected-warning {{uninitialized}}<br>
};<br>
struct C {<br>
- C() {} // expected-note4{{in this constructor}}<br>
+ C() {} // expected-note5{{in this constructor}}<br>
A a1 = a1; // expected-warning {{uninitialized}}<br>
A a2 = a2.get(); // expected-warning {{uninitialized}}<br>
A a3 = a3.num();<br>
@@ -565,8 +582,9 @@ namespace record_fields {<br>
A a7 = const_ref(a7);<br>
A a8 = pointer(&a8);<br>
A a9 = normal(a9); // expected-warning {{uninitialized}}<br>
+ const A a10 = a10; // expected-warning {{uninitialized}}<br>
};<br>
- struct D { // expected-note4{{in the implicit default constructor}}<br>
+ struct D { // expected-note5{{in the implicit default constructor}}<br>
A a1 = a1; // expected-warning {{uninitialized}}<br>
A a2 = a2.get(); // expected-warning {{uninitialized}}<br>
A a3 = a3.num();<br>
@@ -576,6 +594,7 @@ namespace record_fields {<br>
A a7 = const_ref(a7);<br>
A a8 = pointer(&a8);<br>
A a9 = normal(a9); // expected-warning {{uninitialized}}<br>
+ const A a10 = a10; // expected-warning {{uninitialized}}<br>
};<br>
D d;<br>
struct E {<br>
@@ -588,6 +607,7 @@ namespace record_fields {<br>
A a7 = const_ref(a7);<br>
A a8 = pointer(&a8);<br>
A a9 = normal(a9);<br>
+ const A a10 = a10;<br>
};<br>
}<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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>