From: Richard Trieu Date: Tue, 23 Sep 2014 22:52:42 +0000 (+0000) Subject: Improve -Wuninitialized to take into account field ordering with initializer X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=86dc7fea0b15eac18ce418886d8ac32a2095f7a6;p=clang Improve -Wuninitialized to take into account field ordering with initializer lists. Since the fields are inititalized one at a time, using a field with lower index to initialize a higher indexed field should not be warned on. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@218339 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index ae79720e38..b3098abbb0 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -8240,6 +8240,8 @@ namespace { bool isPODType; bool isReferenceType; + bool isInitList; + llvm::SmallVector InitFieldIndex; public: typedef EvaluatedExprVisitor Inherited; @@ -8248,6 +8250,7 @@ namespace { isPODType = false; isRecordType = false; isReferenceType = false; + isInitList = false; if (ValueDecl *VD = dyn_cast(OrigDecl)) { isPODType = VD->getType().isPODType(S.Context); isRecordType = VD->getType()->isRecordType(); @@ -8255,6 +8258,78 @@ namespace { } } + // For most expressions, just call the visitor. For initializer lists, + // track the index of the field being initialized since fields are + // initialized in order allowing use of previously initialized fields. + void CheckExpr(Expr *E) { + InitListExpr *InitList = dyn_cast(E); + if (!InitList) { + Visit(E); + return; + } + + // Track and increment the index here. + isInitList = true; + InitFieldIndex.push_back(0); + for (auto Child : InitList->children()) { + CheckExpr(cast(Child)); + ++InitFieldIndex.back(); + } + InitFieldIndex.pop_back(); + } + + // Returns true if MemberExpr is checked and no futher checking is needed. + // Returns false if additional checking is required. + bool CheckInitListMemberExpr(MemberExpr *E, bool CheckReference) { + llvm::SmallVector Fields; + Expr *Base = E; + bool ReferenceField = false; + + // Get the field memebers used. + while (MemberExpr *ME = dyn_cast(Base)) { + FieldDecl *FD = dyn_cast(ME->getMemberDecl()); + if (!FD) + return false; + Fields.push_back(FD); + if (FD->getType()->isReferenceType()) + ReferenceField = true; + Base = ME->getBase()->IgnoreParenImpCasts(); + } + + // Keep checking only if the base Decl is the same. + DeclRefExpr *DRE = dyn_cast(Base); + if (!DRE || DRE->getDecl() != OrigDecl) + return false; + + // A reference field can be bound to an unininitialized field. + if (CheckReference && !ReferenceField) + return true; + + // Convert FieldDecls to their index number. + llvm::SmallVector UsedFieldIndex; + for (auto I = Fields.rbegin(), E = Fields.rend(); I != E; ++I) { + UsedFieldIndex.push_back((*I)->getFieldIndex()); + } + + // See if a warning is needed by checking the first difference in index + // numbers. If field being used has index less than the field being + // initialized, then the use is safe. + for (auto UsedIter = UsedFieldIndex.begin(), + UsedEnd = UsedFieldIndex.end(), + OrigIter = InitFieldIndex.begin(), + OrigEnd = InitFieldIndex.end(); + UsedIter != UsedEnd && OrigIter != OrigEnd; ++UsedIter, ++OrigIter) { + if (*UsedIter < *OrigIter) + return true; + if (*UsedIter > *OrigIter) + break; + } + + // TODO: Add a different warning which will print the field names. + HandleDeclRefExpr(DRE); + return true; + } + // For most expressions, the cast is directly above the DeclRefExpr. // For conditional operators, the cast can be outside the conditional // operator if both expressions are DeclRefExpr's. @@ -8290,6 +8365,12 @@ namespace { } if (isa(E)) { + if (isInitList) { + if (CheckInitListMemberExpr(cast(E), + false /*CheckReference*/)) + return; + } + Expr *Base = E->IgnoreParenImpCasts(); while (MemberExpr *ME = dyn_cast(Base)) { // Check for static member variables and don't warn on them. @@ -8323,6 +8404,11 @@ namespace { } void VisitMemberExpr(MemberExpr *E) { + if (isInitList) { + if (CheckInitListMemberExpr(E, true /*CheckReference*/)) + return; + } + // Don't warn on arrays since they can be treated as pointers. if (E->getType()->canDecayToPointerType()) return; @@ -8439,7 +8525,7 @@ namespace { if (DRE->getDecl() == OrigDecl) return; - SelfReferenceChecker(S, OrigDecl).Visit(E); + SelfReferenceChecker(S, OrigDecl).CheckExpr(E); } } diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp index 336fea87a5..24a9d670e8 100644 --- a/test/SemaCXX/constant-expression-cxx11.cpp +++ b/test/SemaCXX/constant-expression-cxx11.cpp @@ -602,7 +602,7 @@ struct A { constexpr A(int a, int b) : k(a + b) {} int k; }; constexpr int fn(const A &a) { return a.k; } static_assert(fn(A(4,5)) == 9, ""); -struct B { int n; int m; } constexpr b = { 0, b.n }; // expected-warning {{uninitialized}} +struct B { int n; int m; } constexpr b = { 0, b.n }; struct C { constexpr C(C *this_) : m(42), n(this_->m) {} // ok int m, n; diff --git a/test/SemaCXX/constant-expression-cxx1y.cpp b/test/SemaCXX/constant-expression-cxx1y.cpp index 521526ddba..e1929f7637 100644 --- a/test/SemaCXX/constant-expression-cxx1y.cpp +++ b/test/SemaCXX/constant-expression-cxx1y.cpp @@ -719,8 +719,7 @@ namespace deduced_return_type { namespace modify_temporary_during_construction { struct A { int &&temporary; int x; int y; }; constexpr int f(int &r) { r *= 9; return r - 12; } - // FIXME: The 'uninitialized' warning here is bogus. - constexpr A a = { 6, f(a.temporary), a.temporary }; // expected-warning {{uninitialized}} expected-note {{temporary created here}} + constexpr A a = { 6, f(a.temporary), a.temporary }; // expected-note {{temporary created here}} static_assert(a.x == 42, ""); static_assert(a.y == 54, ""); constexpr int k = a.temporary++; // expected-error {{constant expression}} expected-note {{outside the expression that created the temporary}} diff --git a/test/SemaCXX/uninitialized.cpp b/test/SemaCXX/uninitialized.cpp index bb2ecabfc8..c57ee2fc37 100644 --- a/test/SemaCXX/uninitialized.cpp +++ b/test/SemaCXX/uninitialized.cpp @@ -877,3 +877,56 @@ namespace delegating_constructor { int x; }; } + +namespace init_list { + int num = 5; + struct A { int i1, i2; }; + struct B { A a1, a2; }; + + A a1{1,2}; + A a2{a2.i1 + 2}; // expected-warning{{uninitialized}} + A a3 = {a3.i1 + 2}; // expected-warning{{uninitialized}} + A a4 = A{a4.i2 + 2}; // expected-warning{{uninitialized}} + + B b1 = { {}, {} }; + B b2 = { {}, b2.a1 }; + B b3 = { b3.a1 }; // expected-warning{{uninitialized}} + B b4 = { {}, b4.a2} ; // expected-warning{{uninitialized}} + B b5 = { b5.a2 }; // expected-warning{{uninitialized}} + + B b6 = { {b6.a1.i1} }; // expected-warning{{uninitialized}} + B b7 = { {0, b7.a1.i1} }; + B b8 = { {}, {b8.a1.i1} }; + B b9 = { {}, {0, b9.a1.i1} }; + + B b10 = { {b10.a1.i2} }; // expected-warning{{uninitialized}} + B b11 = { {0, b11.a1.i2} }; // expected-warning{{uninitialized}} + B b12 = { {}, {b12.a1.i2} }; + B b13 = { {}, {0, b13.a1.i2} }; + + B b14 = { {b14.a2.i1} }; // expected-warning{{uninitialized}} + B b15 = { {0, b15.a2.i1} }; // expected-warning{{uninitialized}} + B b16 = { {}, {b16.a2.i1} }; // expected-warning{{uninitialized}} + B b17 = { {}, {0, b17.a2.i1} }; + + B b18 = { {b18.a2.i2} }; // expected-warning{{uninitialized}} + B b19 = { {0, b19.a2.i2} }; // expected-warning{{uninitialized}} + B b20 = { {}, {b20.a2.i2} }; // expected-warning{{uninitialized}} + B b21 = { {}, {0, b21.a2.i2} }; // expected-warning{{uninitialized}} + + B b22 = { {b18.a2.i2 + 5} }; + + struct C {int a; int& b; int c; }; + C c1 = { 0, num, 0 }; + C c2 = { 1, num, c2.b }; + C c3 = { c3.b, num }; // expected-warning{{uninitialized}} + C c4 = { 0, c4.b, 0 }; // expected-warning{{uninitialized}} + C c5 = { 0, c5.c, 0 }; + C c6 = { c6.b, num, 0 }; // expected-warning{{uninitialized}} + C c7 = { 0, c7.a, 0 }; + + struct D {int &a; int &b; }; + D d1 = { num, num }; + D d2 = { num, d2.a }; + D d3 = { d3.b, num }; // expected-warning{{uninitialized}} +};