From ef64e94d5c14281a3b354b08a69f3400d47896f0 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Fri, 25 Oct 2013 00:56:00 +0000 Subject: Simplify and refactor the uninitialized field warning. Change the uninitialized field warnings so that field initializers are checked inside the constructor. Previously, in class initializers were checked separately. Running one set of checks also simplifies the logic for preventing duplicate warnings. Added new checks to warn when an uninitialized field is used in base class initialization. Also fixed misspelling of uninitialized and moved all code for this warning together. llvm-svn: 193386 --- clang/lib/Sema/SemaDeclCXX.cpp | 280 +++++++++++++---------------------------- 1 file changed, 90 insertions(+), 190 deletions(-) (limited to 'clang/lib/Sema/SemaDeclCXX.cpp') diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 221c53f03f6..3b1f67c07fe 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -2110,40 +2110,20 @@ namespace { class UninitializedFieldVisitor : public EvaluatedExprVisitor { 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. + // List of Decls to generate a warning on. Also remove Decls that become + // initialized. llvm::SmallPtrSet &Decls; - bool WarnOnSelfReference; // If non-null, add a note to the warning pointing back to the constructor. const CXXConstructorDecl *Constructor; public: typedef EvaluatedExprVisitor Inherited; - UninitializedFieldVisitor(Sema &S, ValueDecl *VD, + UninitializedFieldVisitor(Sema &S, llvm::SmallPtrSet &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(VD)) - this->VD = IFD->getAnonField(); - else - this->VD = VD; - isReferenceType = this->VD->getType()->isReferenceType(); - } - } + : Inherited(S.Context), S(S), Decls(Decls), + Constructor(Constructor) { } void HandleMemberExpr(MemberExpr *ME, bool CheckReferenceOnly) { - if (!VD) - return; - - if (CheckReferenceOnly && !isReferenceType) - return; - if (isa(ME->getMemberDecl())) return; @@ -2170,36 +2150,27 @@ namespace { 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); + if (!Decls.count(FoundVD)) return; - } - if (CheckReferenceOnly) + const bool IsReference = FoundVD->getType()->isReferenceType(); + + // Prevent double warnings on use of unbounded references. + if (IsReference != 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); + unsigned diag = IsReference + ? diag::warn_reference_field_is_uninit + : diag::warn_field_is_uninit; + S.Diag(FieldME->getExprLoc(), diag) << FoundVD; + if (Constructor) + S.Diag(Constructor->getLocation(), + diag::note_uninit_in_this_constructor) + << (Constructor->isDefaultConstructor() && Constructor->isImplicit()); - } } void HandleValue(Expr *E) { - if (!VD) - return; - E = E->IgnoreParens(); if (MemberExpr *ME = dyn_cast(E)) { @@ -2236,6 +2207,7 @@ namespace { } void VisitMemberExpr(MemberExpr *ME) { + // All uses of unbounded reference fields will warn. HandleMemberExpr(ME, true /*CheckReferenceOnly*/); Inherited::VisitMemberExpr(ME); @@ -2272,20 +2244,78 @@ namespace { if (E->getOpcode() == BO_Assign) if (MemberExpr *ME = dyn_cast(E->getLHS())) if (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) - Decls.erase(FD); + if (!FD->getType()->isReferenceType()) + Decls.erase(FD); Inherited::VisitBinaryOperator(E); } }; static void CheckInitExprContainsUninitializedFields( - Sema &S, Expr *E, ValueDecl *VD, llvm::SmallPtrSet &Decls, - bool WarnOnSelfReference, const CXXConstructorDecl *Constructor = 0) { - if (Decls.size() == 0 && !WarnOnSelfReference) + Sema &S, Expr *E, llvm::SmallPtrSet &Decls, + const CXXConstructorDecl *Constructor) { + if (Decls.size() == 0) return; - if (E) - UninitializedFieldVisitor(S, VD, Decls, WarnOnSelfReference, Constructor) - .Visit(E); + if (!E) + return; + + if (CXXDefaultInitExpr *Default = dyn_cast(E)) { + E = Default->getExpr(); + if (!E) + return; + // In class initializers will point to the constructor. + UninitializedFieldVisitor(S, Decls, Constructor).Visit(E); + } else { + UninitializedFieldVisitor(S, Decls, 0).Visit(E); + } + } + + // 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. + static void DiagnoseUninitializedFields( + Sema &SemaRef, const CXXConstructorDecl *Constructor) { + + if (SemaRef.getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, + Constructor->getLocation()) + == DiagnosticsEngine::Ignored) { + return; + } + + if (Constructor->isInvalidDecl()) + return; + + const CXXRecordDecl *RD = Constructor->getParent(); + + // Holds fields that are uninitialized. + llvm::SmallPtrSet UninitializedFields; + + // At the beginning, all fields are uninitialized. + for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end(); + I != E; ++I) { + if (FieldDecl *FD = dyn_cast(*I)) { + UninitializedFields.insert(FD); + } else if (IndirectFieldDecl *IFD = dyn_cast(*I)) { + UninitializedFields.insert(IFD->getAnonField()); + } + } + + for (CXXConstructorDecl::init_const_iterator FieldInit = + Constructor->init_begin(), + FieldInitEnd = Constructor->init_end(); + FieldInit != FieldInitEnd; ++FieldInit) { + + Expr *InitExpr = (*FieldInit)->getInit(); + + CheckInitExprContainsUninitializedFields( + SemaRef, InitExpr, UninitializedFields, Constructor); + + if (FieldDecl *Field = (*FieldInit)->getAnyMember()) + UninitializedFields.erase(Field); + } } } // namespace @@ -3779,90 +3809,6 @@ 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. -static void DiagnoseUnitializedFields( - Sema &SemaRef, const CXXConstructorDecl *Constructor) { - - if (SemaRef.getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, - Constructor->getLocation()) - == DiagnosticsEngine::Ignored) { - return; - } - - const CXXRecordDecl *RD = Constructor->getParent(); - - // Holds fields that are uninitialized. - llvm::SmallPtrSet UninitializedFields; - - for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end(); - I != E; ++I) { - if (FieldDecl *FD = dyn_cast(*I)) { - UninitializedFields.insert(FD); - } else if (IndirectFieldDecl *IFD = dyn_cast(*I)) { - UninitializedFields.insert(IFD->getAnonField()); - } - } - - // Fields already checked when processing the in class initializers. - llvm::SmallPtrSet - InClassUninitializedFields = UninitializedFields; - - for (CXXConstructorDecl::init_const_iterator FieldInit = - Constructor->init_begin(), - FieldInitEnd = Constructor->init_end(); - FieldInit != FieldInitEnd; ++FieldInit) { - - FieldDecl *Field = (*FieldInit)->getAnyMember(); - Expr *InitExpr = (*FieldInit)->getInit(); - - if (!Field) { - CheckInitExprContainsUninitializedFields( - SemaRef, InitExpr, 0, UninitializedFields, - false/*WarnOnSelfReference*/); - continue; - } - - if (CXXDefaultInitExpr *Default = dyn_cast(InitExpr)) { - // This field is initialized with an in-class initailzer. Remove the - // fields already checked to prevent duplicate warnings. - llvm::SmallPtrSet DiffSet = UninitializedFields; - for (llvm::SmallPtrSet::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); - } -} - /// ActOnMemInitializers - Handle the member initializers for a constructor. void Sema::ActOnMemInitializers(Decl *ConstructorDecl, SourceLocation ColonLoc, @@ -3928,7 +3874,7 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl, SetCtorInitializers(Constructor, AnyErrors, MemInits); - DiagnoseUnitializedFields(*this, Constructor); + DiagnoseUninitializedFields(*this, Constructor); } void @@ -4056,8 +4002,10 @@ void Sema::ActOnDefaultCtorInitializers(Decl *CDtorDecl) { return; if (CXXConstructorDecl *Constructor - = dyn_cast(CDtorDecl)) + = dyn_cast(CDtorDecl)) { SetCtorInitializers(Constructor, /*AnyErrors=*/false); + DiagnoseUninitializedFields(*this, Constructor); + } } bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T, @@ -8215,61 +8163,13 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation, if (ASTMutationListener *L = getASTMutationListener()) { L->CompletedImplicitDefinition(Constructor); } + + DiagnoseUninitializedFields(*this, Constructor); } void Sema::ActOnFinishDelayedMemberInitializers(Decl *D) { // Perform any delayed checks on exception specifications. CheckDelayedMemberExceptionSpecs(); - - // 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(D); - if (!RD) return; - - // Holds fields that are uninitialized. - llvm::SmallPtrSet 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(*I)) { - UninitializedFields.insert(FD); - } else if (IndirectFieldDecl *IFD = dyn_cast(*I)) { - UninitializedFields.insert(IFD->getAnonField()); - } - } - - for (DeclContext::decl_iterator I = RD->decls_begin(), E = RD->decls_end(); - I != E; ++I) { - FieldDecl *FD = dyn_cast(*I); - if (!FD) - if (IndirectFieldDecl *IFD = dyn_cast(*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 { -- cgit v1.2.3