r190657 - Refactor the uninitialized field visitor. Also moved the calls to the visitor
Richard Trieu
rtrieu at google.com
Thu Sep 12 20:20:53 PDT 2013
Author: rtrieu
Date: Thu Sep 12 22:20:53 2013
New Revision: 190657
URL: http://llvm.org/viewvc/llvm-project?rev=190657&view=rev
Log:
Refactor the uninitialized field visitor. Also moved the calls to the visitor
later in the code so that the expressions will have addition processing first.
This catches a few additional cases of uninitialized uses of class fields.
Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/uninitialized.cpp
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=190657&r1=190656&r2=190657&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 12 22:20:53 2013
@@ -2073,6 +2073,7 @@ namespace {
: public EvaluatedExprVisitor<UninitializedFieldVisitor> {
Sema &S;
ValueDecl *VD;
+ bool isReferenceType;
public:
typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
@@ -2081,48 +2082,44 @@ namespace {
this->VD = IFD->getAnonField();
else
this->VD = VD;
+ isReferenceType = this->VD->getType()->isReferenceType();
}
- void HandleExpr(Expr *E) {
- if (!E) return;
+ void HandleMemberExpr(MemberExpr *ME) {
+ if (isa<EnumConstantDecl>(ME->getMemberDecl()))
+ return;
+
+ // FieldME is the inner-most MemberExpr that is not an anonymous struct
+ // or union.
+ MemberExpr *FieldME = ME;
+
+ Expr *Base = ME;
+ while (isa<MemberExpr>(Base)) {
+ ME = cast<MemberExpr>(Base);
+
+ if (isa<VarDecl>(ME->getMemberDecl()))
+ return;
+
+ if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+ if (!FD->isAnonymousStructOrUnion())
+ FieldME = ME;
- // Expressions like x(x) sometimes lack the surrounding expressions
- // but need to be checked anyways.
- HandleValue(E);
- Visit(E);
+ Base = ME->getBase();
+ }
+
+ if (VD == FieldME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
+ unsigned diag = VD->getType()->isReferenceType()
+ ? diag::warn_reference_field_is_uninit
+ : diag::warn_field_is_uninit;
+ S.Diag(FieldME->getExprLoc(), diag) << VD;
+ }
}
void HandleValue(Expr *E) {
E = E->IgnoreParens();
if (MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
- if (isa<EnumConstantDecl>(ME->getMemberDecl()))
- return;
-
- // FieldME is the inner-most MemberExpr that is not an anonymous struct
- // or union.
- MemberExpr *FieldME = ME;
-
- Expr *Base = E;
- while (isa<MemberExpr>(Base)) {
- ME = cast<MemberExpr>(Base);
-
- if (isa<VarDecl>(ME->getMemberDecl()))
- return;
-
- if (FieldDecl *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
- if (!FD->isAnonymousStructOrUnion())
- FieldME = ME;
-
- Base = ME->getBase();
- }
-
- if (VD == FieldME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
- unsigned diag = VD->getType()->isReferenceType()
- ? diag::warn_reference_field_is_uninit
- : diag::warn_field_is_uninit;
- S.Diag(FieldME->getExprLoc(), diag) << VD;
- }
+ HandleMemberExpr(ME);
return;
}
@@ -2154,6 +2151,13 @@ namespace {
}
}
+ void VisitMemberExpr(MemberExpr *ME) {
+ if (isReferenceType)
+ HandleMemberExpr(ME);
+
+ Inherited::VisitMemberExpr(ME);
+ }
+
void VisitImplicitCastExpr(ImplicitCastExpr *E) {
if (E->getCastKind() == CK_LValueToRValue)
HandleValue(E->getSubExpr());
@@ -2161,6 +2165,16 @@ namespace {
Inherited::VisitImplicitCastExpr(E);
}
+ void VisitCXXConstructExpr(CXXConstructExpr *E) {
+ if (E->getNumArgs() == 1)
+ if (ImplicitCastExpr* ICE = dyn_cast<ImplicitCastExpr>(E->getArg(0)))
+ if (ICE->getCastKind() == CK_NoOp)
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(ICE->getSubExpr()))
+ HandleMemberExpr(ME);
+
+ Inherited::VisitCXXConstructExpr(E);
+ }
+
void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
Expr *Callee = E->getCallee();
if (isa<MemberExpr>(Callee))
@@ -2171,7 +2185,8 @@ namespace {
};
static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
ValueDecl *VD) {
- UninitializedFieldVisitor(S, VD).HandleExpr(E);
+ if (E)
+ UninitializedFieldVisitor(S, VD).Visit(E);
}
} // namespace
@@ -2198,11 +2213,6 @@ Sema::ActOnCXXInClassMemberInitializer(D
return;
}
- if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, InitLoc)
- != DiagnosticsEngine::Ignored) {
- CheckInitExprContainsUninitializedFields(*this, InitExpr, FD);
- }
-
ExprResult Init = InitExpr;
if (!FD->getType()->isDependentType() && !InitExpr->isTypeDependent()) {
InitializedEntity Entity = InitializedEntity::InitializeMember(FD);
@@ -2228,6 +2238,11 @@ Sema::ActOnCXXInClassMemberInitializer(D
InitExpr = Init.release();
+ if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, InitLoc)
+ != DiagnosticsEngine::Ignored) {
+ CheckInitExprContainsUninitializedFields(*this, InitExpr, FD);
+ }
+
FD->setInClassInitializer(InitExpr);
}
@@ -2555,10 +2570,6 @@ Sema::BuildMemberInitializer(ValueDecl *
if (Member->isInvalidDecl())
return true;
- // Diagnose value-uses of fields to initialize themselves, e.g.
- // foo(foo)
- // where foo is not also a parameter to the constructor.
- // TODO: implement -Wuninitialized and fold this into that framework.
MultiExprArg Args;
if (ParenListExpr *ParenList = dyn_cast<ParenListExpr>(Init)) {
Args = MultiExprArg(ParenList->getExprs(), ParenList->getNumExprs());
@@ -2569,19 +2580,6 @@ Sema::BuildMemberInitializer(ValueDecl *
Args = Init;
}
- if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
- != DiagnosticsEngine::Ignored)
- for (unsigned i = 0, e = Args.size(); i != e; ++i)
- // 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.
- CheckInitExprContainsUninitializedFields(*this, Args[i], Member);
-
SourceRange InitRange = Init->getSourceRange();
if (Member->getType()->isDependentType() || Init->isTypeDependent()) {
@@ -2621,6 +2619,23 @@ Sema::BuildMemberInitializer(ValueDecl *
Init = MemberInit.get();
}
+ // Diagnose value-uses of fields to initialize themselves, e.g.
+ // foo(foo)
+ // where foo is not also a parameter to the constructor.
+ // 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.
+ if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
+ != DiagnosticsEngine::Ignored) {
+ CheckInitExprContainsUninitializedFields(*this, Init, Member);
+ }
+
if (DirectMember) {
return new (Context) CXXCtorInitializer(Context, DirectMember, IdLoc,
InitRange.getBegin(), Init,
Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=190657&r1=190656&r2=190657&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
+++ cfe/trunk/test/SemaCXX/uninitialized.cpp Thu Sep 12 22:20:53 2013
@@ -523,3 +523,41 @@ namespace lambdas {
A a2([&] { return a2.x; }); // ok
}
}
+
+namespace record_fields {
+ struct A {
+ A() {}
+ A get();
+ static A num();
+ static A copy(A);
+ static A something(A&);
+ };
+
+ A ref(A&);
+ A const_ref(const A&);
+ A pointer(A*);
+ A normal(A);
+
+ struct B {
+ A a;
+ B(char (*)[1]) : a(a) {} // expected-warning {{uninitialized}}
+ B(char (*)[2]) : a(a.get()) {} // expected-warning {{uninitialized}}
+ B(char (*)[3]) : a(a.num()) {}
+ B(char (*)[4]) : a(a.copy(a)) {} // expected-warning {{uninitialized}}
+ B(char (*)[5]) : a(a.something(a)) {}
+ B(char (*)[6]) : a(ref(a)) {}
+ B(char (*)[7]) : a(const_ref(a)) {}
+ B(char (*)[8]) : a(pointer(&a)) {}
+ B(char (*)[9]) : a(normal(a)) {} // expected-warning {{uninitialized}}
+
+ A a1 = a1; // expected-warning {{uninitialized}}
+ A a2 = a2.get(); // expected-warning {{uninitialized}}
+ A a3 = a3.num();
+ A a4 = a4.copy(a4); // expected-warning {{uninitialized}}
+ A a5 = a5.something(a5);
+ A a6 = ref(a6);
+ A a7 = const_ref(a7);
+ A a8 = pointer(&a8);
+ A a9 = normal(a9); // expected-warning {{uninitialized}}
+ };
+}
More information about the cfe-commits
mailing list