r215471 - Improve -Wuninitialized to catch const classes being used in their own copy
Richard Smith
richard at metafoo.co.uk
Sat Aug 30 16:24:05 PDT 2014
On Mon, Aug 25, 2014 at 9:49 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Tue, Aug 12, 2014 at 4:19 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Tue, Aug 12, 2014 at 2:05 PM, Richard Trieu <rtrieu at google.com> 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140830/517006c8/attachment.html>
More information about the cfe-commits
mailing list