r215471 - Improve -Wuninitialized to catch const classes being used in their own copy

Richard Trieu rtrieu at google.com
Mon Aug 25 21:49:03 PDT 2014


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.

>
>

>
>> +        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/20140825/52b88fbb/attachment.html>


More information about the cfe-commits mailing list