On Tue, Aug 12, 2014 at 4:19 PM, Richard Smith <[email protected]> wrote:
> 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 > > r216438 now treats std::move arguments as used. It will warn on this case now. > ... or > > S::S(T x) : x_(x_) {} // oops > I couldn't get this syntax to trigger the move constructor, only the copy constructor. > > > >> + 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
