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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190657 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 594f798..c45548d 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2073,6 +2073,7 @@
: 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 @@
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;
- // Expressions like x(x) sometimes lack the surrounding expressions
- // but need to be checked anyways.
- HandleValue(E);
- Visit(E);
+ // 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;
+
+ 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 @@
}
}
+ 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 @@
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 @@
};
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 @@
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 @@
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 @@
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 @@
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 @@
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,