On Mon, Aug 25, 2014 at 9:49 PM, Richard Trieu <[email protected]> wrote:
> 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. > Sorry, I forgot the std::move(...) here. =) + 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
