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