r191068 - Modify the uninitialized field visitor to detect uninitialized use across the
Richard Trieu
rtrieu at google.com
Thu Sep 19 20:03:06 PDT 2013
Author: rtrieu
Date: Thu Sep 19 22:03:06 2013
New Revision: 191068
URL: http://llvm.org/viewvc/llvm-project?rev=191068&view=rev
Log:
Modify the uninitialized field visitor to detect uninitialized use across the
fields in the class. This allows a better checking of member intiailizers and
in class initializers in regards to initialization ordering.
For instance, this code will now produce warnings:
class A {
int x;
int y;
A() : x(y) {} // y is initialized after x, warn here
A(int): y(x) {} // default initialization of leaves x uninitialized, warn here
};
Several test cases were updated with -Wno-uninitialized to silence this warning.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp
cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp
cfe/trunk/test/SemaCXX/constexpr-value-init.cpp
cfe/trunk/test/SemaCXX/cxx0x-class.cpp
cfe/trunk/test/SemaCXX/uninitialized.cpp
cfe/trunk/test/SemaCXX/warn-unused-private-field.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 19 22:03:06 2013
@@ -1370,6 +1370,8 @@ def warn_field_is_uninit : Warning<"fiel
def warn_reference_field_is_uninit : Warning<
"reference %0 is not yet bound to a value when used here">,
InGroup<Uninitialized>;
+def note_uninit_in_this_constructor : Note<
+ "during field initialization in this constructor">;
def warn_static_self_reference_in_init : Warning<
"static variable %0 is suspiciously used within its own initialization">,
InGroup<UninitializedStaticSelfInit>;
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 19 22:03:06 2013
@@ -2081,20 +2081,37 @@ namespace {
class UninitializedFieldVisitor
: public EvaluatedExprVisitor<UninitializedFieldVisitor> {
Sema &S;
+ // If VD is null, this visitor will only update the Decls set.
ValueDecl *VD;
bool isReferenceType;
+ // List of Decls to generate a warning on.
+ llvm::SmallPtrSet<ValueDecl*, 4> &Decls;
+ bool WarnOnSelfReference;
+ // If non-null, add a note to the warning pointing back to the constructor.
+ const CXXConstructorDecl *Constructor;
public:
typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
- UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
- S(S) {
- if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(VD))
- this->VD = IFD->getAnonField();
- else
- this->VD = VD;
- isReferenceType = this->VD->getType()->isReferenceType();
+ UninitializedFieldVisitor(Sema &S, ValueDecl *VD,
+ llvm::SmallPtrSet<ValueDecl*, 4> &Decls,
+ bool WarnOnSelfReference,
+ const CXXConstructorDecl *Constructor)
+ : Inherited(S.Context), S(S), VD(VD), isReferenceType(false), Decls(Decls),
+ WarnOnSelfReference(WarnOnSelfReference), Constructor(Constructor) {
+ // When VD is null, this visitor is used to detect initialization of other
+ // fields.
+ if (VD) {
+ if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(VD))
+ this->VD = IFD->getAnonField();
+ else
+ this->VD = VD;
+ isReferenceType = this->VD->getType()->isReferenceType();
+ }
}
void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) {
+ if (!VD)
+ return;
+
if (CheckReferenceOnly && !isReferenceType)
return;
@@ -2122,15 +2139,38 @@ namespace {
if (!isa<CXXThisExpr>(Base))
return;
- if (VD == FieldME->getMemberDecl()) {
+ ValueDecl* FoundVD = FieldME->getMemberDecl();
+
+ if (VD == FoundVD) {
+ if (!WarnOnSelfReference)
+ return;
+
unsigned diag = isReferenceType
? diag::warn_reference_field_is_uninit
: diag::warn_field_is_uninit;
S.Diag(FieldME->getExprLoc(), diag) << VD;
+ if (Constructor)
+ S.Diag(Constructor->getLocation(),
+ diag::note_uninit_in_this_constructor);
+ return;
+ }
+
+ if (CheckReferenceOnly)
+ return;
+
+ if (Decls.count(FoundVD)) {
+ S.Diag(FieldME->getExprLoc(), diag::warn_field_is_uninit) << FoundVD;
+ if (Constructor)
+ S.Diag(Constructor->getLocation(),
+ diag::note_uninit_in_this_constructor);
+
}
}
void HandleValue(Expr *E) {
+ if (!VD)
+ return;
+
E = E->IgnoreParens();
if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
@@ -2180,7 +2220,7 @@ namespace {
}
void VisitCXXConstructExpr(CXXConstructExpr *E) {
- if (E->getNumArgs() == 1)
+ 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()))
@@ -2196,11 +2236,27 @@ namespace {
Inherited::VisitCXXMemberCallExpr(E);
}
+
+ void VisitBinaryOperator(BinaryOperator *E) {
+ // If a field assignment is detected, remove the field from the
+ // uninitiailized field set.
+ if (E->getOpcode() == BO_Assign)
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(E->getLHS()))
+ if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+ Decls.erase(FD);
+
+ Inherited::VisitBinaryOperator(E);
+ }
};
- static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
- ValueDecl *VD) {
+ static void CheckInitExprContainsUninitializedFields(
+ Sema &S, Expr *E, ValueDecl *VD, llvm::SmallPtrSet<ValueDecl*, 4> &Decls,
+ bool WarnOnSelfReference, const CXXConstructorDecl *Constructor = 0) {
+ if (Decls.size() == 0 && !WarnOnSelfReference)
+ return;
+
if (E)
- UninitializedFieldVisitor(S, VD).Visit(E);
+ UninitializedFieldVisitor(S, VD, Decls, WarnOnSelfReference, Constructor)
+ .Visit(E);
}
} // namespace
@@ -2252,11 +2308,6 @@ Sema::ActOnCXXInClassMemberInitializer(D
InitExpr = Init.release();
- if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, InitLoc)
- != DiagnosticsEngine::Ignored) {
- CheckInitExprContainsUninitializedFields(*this, InitExpr, FD);
- }
-
FD->setInClassInitializer(InitExpr);
}
@@ -3702,15 +3753,9 @@ bool CheckRedundantUnionInit(Sema &S,
// Diagnose value-uses of fields to initialize themselves, e.g.
// foo(foo)
// where foo is not also a parameter to the constructor.
+// Also diagnose across field uninitialized use such as
+// x(y), y(x)
// TODO: implement -Wuninitialized and fold this into that framework.
-// FIXME: Warn about the case when other fields are used before being
-// initialized. For example, let this field be the i'th field. When
-// initializing the i'th field, throw a warning if any of the >= i'th
-// fields are used, as they are not yet initialized.
-// Right now we are only handling the case where the i'th field uses
-// itself in its initializer.
-// Also need to take into account that some fields may be initialized by
-// in-class initializers, see C++11 [class.base.init]p9.
static void DiagnoseUnitializedFields(
Sema &SemaRef, const CXXConstructorDecl *Constructor) {
@@ -3720,19 +3765,72 @@ static void DiagnoseUnitializedFields(
return;
}
- for (CXXConstructorDecl::init_const_iterator
- FieldInit = Constructor->init_begin(),
+ const CXXRecordDecl *RD = Constructor->getParent();
+
+ // Holds fields that are uninitialized.
+ llvm::SmallPtrSet<ValueDecl*, 4> UninitializedFields;
+
+ for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+ I != E; ++I) {
+ if (FieldDecl *FD = dyn_cast<FieldDecl>(*I)) {
+ UninitializedFields.insert(FD);
+ } else if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I)) {
+ UninitializedFields.insert(IFD->getAnonField());
+ }
+ }
+
+ // Fields already checked when processing the in class initializers.
+ llvm::SmallPtrSet<ValueDecl*, 4>
+ InClassUninitializedFields = UninitializedFields;
+
+ for (CXXConstructorDecl::init_const_iterator FieldInit =
+ Constructor->init_begin(),
FieldInitEnd = Constructor->init_end();
FieldInit != FieldInitEnd; ++FieldInit) {
- FieldDecl *FD = (*FieldInit)->getAnyMember();
- if (!FD)
- continue;
+ FieldDecl *Field = (*FieldInit)->getAnyMember();
Expr *InitExpr = (*FieldInit)->getInit();
- if (!isa<CXXDefaultInitExpr>(InitExpr)) {
- CheckInitExprContainsUninitializedFields(SemaRef, InitExpr, FD);
+ if (!Field) {
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, InitExpr, 0, UninitializedFields,
+ false/*WarnOnSelfReference*/);
+ continue;
+ }
+
+ if (CXXDefaultInitExpr *Default = dyn_cast<CXXDefaultInitExpr>(InitExpr)) {
+ // This field is initialized with an in-class initailzer. Remove the
+ // fields already checked to prevent duplicate warnings.
+ llvm::SmallPtrSet<ValueDecl*, 4> DiffSet = UninitializedFields;
+ for (llvm::SmallPtrSet<ValueDecl*, 4>::iterator
+ I = InClassUninitializedFields.begin(),
+ E = InClassUninitializedFields.end();
+ I != E; ++I) {
+ DiffSet.erase(*I);
+ }
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, Default->getExpr(), Field, DiffSet,
+ DiffSet.count(Field), Constructor);
+
+ // Update the unitialized field sets.
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, Default->getExpr(), 0, UninitializedFields,
+ false);
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, Default->getExpr(), 0, InClassUninitializedFields,
+ false);
+ } else {
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, InitExpr, Field, UninitializedFields,
+ UninitializedFields.count(Field));
+ if (Expr* InClassInit = Field->getInClassInitializer()) {
+ CheckInitExprContainsUninitializedFields(
+ SemaRef, InClassInit, 0, InClassUninitializedFields,
+ false);
+ }
}
+ UninitializedFields.erase(Field);
+ InClassUninitializedFields.erase(Field);
}
}
@@ -8057,6 +8155,56 @@ void Sema::ActOnFinishDelayedMemberIniti
// Check that any explicitly-defaulted methods have exception specifications
// compatible with their implicit exception specifications.
CheckDelayedExplicitlyDefaultedMemberExceptionSpecs();
+
+ // Once all the member initializers are processed, perform checks to see if
+ // any unintialized use is happeneing.
+ if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit,
+ D->getLocation())
+ == DiagnosticsEngine::Ignored)
+ return;
+
+ CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D);
+ if (!RD) return;
+
+ // Holds fields that are uninitialized.
+ llvm::SmallPtrSet<ValueDecl*, 4> UninitializedFields;
+
+ // In the beginning, every field is uninitialized.
+ for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+ I != E; ++I) {
+ if (FieldDecl *FD = dyn_cast<FieldDecl>(*I)) {
+ UninitializedFields.insert(FD);
+ } else if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I)) {
+ UninitializedFields.insert(IFD->getAnonField());
+ }
+ }
+
+ for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end();
+ I != E; ++I) {
+ FieldDecl *FD = dyn_cast<FieldDecl>(*I);
+ if (!FD)
+ if (IndirectFieldDecl *IFD = dyn_cast<IndirectFieldDecl>(*I))
+ FD = IFD->getAnonField();
+
+ if (!FD)
+ continue;
+
+ Expr *InitExpr = FD->getInClassInitializer();
+ if (!InitExpr) {
+ // Uninitialized reference types will give an error.
+ // Record types with an initializer are default initialized.
+ QualType FieldType = FD->getType();
+ if (FieldType->isReferenceType() || FieldType->isRecordType())
+ UninitializedFields.erase(FD);
+ continue;
+ }
+
+ CheckInitExprContainsUninitializedFields(
+ *this, InitExpr, FD, UninitializedFields,
+ UninitializedFields.count(FD)/*WarnOnSelfReference*/);
+
+ UninitializedFields.erase(FD);
+ }
}
namespace {
Modified: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp (original)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p7.cpp Thu Sep 19 22:03:06 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++1y %s -verify
+// RUN: %clang_cc1 -Wno-uninitialized -std=c++1y %s -verify
// expected-no-diagnostics
Modified: cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp (original)
+++ cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp Thu Sep 19 22:03:06 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -Wno-uninitialized -std=c++11 -verify %s
template<int> struct c { c(int) = delete; typedef void val; operator int() const; };
Modified: cfe/trunk/test/SemaCXX/constexpr-value-init.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constexpr-value-init.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constexpr-value-init.cpp (original)
+++ cfe/trunk/test/SemaCXX/constexpr-value-init.cpp Thu Sep 19 22:03:06 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -std=c++11 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++11 -fsyntax-only -verify
struct A {
constexpr A() : a(b + 1), b(a + 1) {} // expected-note {{outside its lifetime}}
Modified: cfe/trunk/test/SemaCXX/cxx0x-class.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx0x-class.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx0x-class.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx0x-class.cpp Thu Sep 19 22:03:06 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-error=static-float-init %s
+// RUN: %clang_cc1 -Wno-uninitialized -fsyntax-only -verify -std=c++11 -Wno-error=static-float-init %s
int vs = 0;
Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
+++ cfe/trunk/test/SemaCXX/uninitialized.cpp Thu Sep 19 22:03:06 2013
@@ -340,7 +340,10 @@ namespace {
};
struct E {
- int a, b, c;
+ int b = 1;
+ int c = 1;
+ int a; // This field needs to be last to prevent the cross field
+ // uninitialized warning.
E(char (*)[1]) : a(a ? b : c) {} // expected-warning {{field 'a' is uninitialized when used here}}
E(char (*)[2]) : a(b ? a : a) {} // expected-warning 2{{field 'a' is uninitialized when used here}}
E(char (*)[3]) : a(b ? (a) : c) {} // expected-warning {{field 'a' is uninitialized when used here}}
@@ -459,7 +462,7 @@ namespace in_class_initializers {
};
struct U {
- U() : a(b + 1), b(a + 1) {} // FIXME: Warn here.
+ U() : a(b + 1), b(a + 1) {} // expected-warning{{field 'b' is uninitialized when used here}}
int a = 42; // Note: because a and b are in the member initializer list, these initializers are ignored.
int b = 1;
};
@@ -561,3 +564,107 @@ namespace record_fields {
A a9 = normal(a9); // expected-warning {{uninitialized}}
};
}
+
+namespace cross_field_warnings {
+ struct A {
+ int a, b;
+ A() {}
+ A(char (*)[1]) : b(a) {} // expected-warning{{field 'a' is uninitialized when used here}}
+ A(char (*)[2]) : a(b) {} // expected-warning{{field 'b' is uninitialized when used here}}
+ };
+
+ struct B {
+ int a = b; // expected-warning{{field 'b' is uninitialized when used here}}
+ int b;
+ };
+
+ struct C {
+ int a;
+ int b = a; // expected-warning{{field 'a' is uninitialized when used here}}
+ C(char (*)[1]) : a(5) {}
+ C(char (*)[2]) {}
+ };
+
+ struct D {
+ int a;
+ int &b;
+ int &c = a;
+ int d = b;
+ D() : b(a) {}
+ };
+
+ struct E {
+ int a;
+ int get();
+ static int num();
+ E() {}
+ E(int) {}
+ };
+
+ struct F {
+ int a;
+ E e;
+ int b;
+ F(char (*)[1]) : a(e.get()) {} // expected-warning{{field 'e' is uninitialized when used here}}
+ F(char (*)[2]) : a(e.num()) {}
+ F(char (*)[3]) : e(a) {} // expected-warning{{field 'a' is uninitialized when used here}}
+ F(char (*)[4]) : a(4), e(a) {}
+ F(char (*)[5]) : e(b) {} // expected-warning{{field 'b' is uninitialized when used here}}
+ F(char (*)[6]) : e(b), b(4) {} // expected-warning{{field 'b' is uninitialized when used here}}
+ };
+
+ struct G {
+ G(const A&) {};
+ };
+
+ struct H {
+ A a1;
+ G g;
+ A a2;
+ H() : g(a1) {}
+ H(int) : g(a2) {}
+ };
+
+ struct I {
+ I(int*) {}
+ };
+
+ struct J : public I {
+ int *a;
+ int *b;
+ int c;
+ J() : I((a = new int(5))), b(a), c(*a) {}
+ };
+
+ struct K {
+ int a = (b = 5);
+ int b = b + 5;
+ };
+
+ struct L {
+ int a = (b = 5);
+ int b = b + 5; // expected-warning{{field 'b' is uninitialized when used here}}
+ L() : a(5) {} // expected-note{{during field initialization in this constructor}}
+ };
+
+ struct M { };
+
+ struct N : public M {
+ int a;
+ int b;
+ N() : b(a) { } // expected-warning{{field 'a' is uninitialized when used here}}
+ };
+
+ struct O {
+ int x = 42;
+ int get() { return x; }
+ };
+
+ struct P {
+ O o;
+ int x = o.get();
+ P() : x(o.get()) { }
+ };
+
+}
+
Modified: cfe/trunk/test/SemaCXX/warn-unused-private-field.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-private-field.cpp?rev=191068&r1=191067&r2=191068&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unused-private-field.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-private-field.cpp Thu Sep 19 22:03:06 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wunused-private-field -Wused-but-marked-unused -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-private-field -Wused-but-marked-unused -Wno-uninitialized -verify -std=c++11 %s
class NotFullyDefined {
public:
More information about the cfe-commits
mailing list