From 10620eb5164e31208fcbf0437cd79ae535ed0559 Mon Sep 17 00:00:00 2001 From: Sean Hunt Date: Fri, 6 May 2011 20:44:56 +0000 Subject: [PATCH] Modify some deleted function methods to better reflect reality: - New isDefined() function checks for deletedness - isThisDeclarationADefinition checks for deletedness - New doesThisDeclarationHaveABody() does what isThisDeclarationADefinition() used to do - The IsDeleted bit is not propagated across redeclarations - isDeleted() now checks the canoncial declaration - New isDeletedAsWritten() does what it says on the tin. - isUserProvided() now correct (thanks Richard!) This fixes the bug that we weren't catching void foo() = delete; void foo() {} as being a redefinition. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@131013 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Decl.h | 29 +++++++++++++++---- include/clang/AST/DeclCXX.h | 2 +- lib/AST/ASTContext.cpp | 2 +- lib/AST/Decl.cpp | 13 ++++++++- lib/AST/DeclPrinter.cpp | 6 ++-- lib/AST/DumpXML.cpp | 4 +-- lib/CodeGen/CodeGenModule.cpp | 4 +-- lib/CodeGen/ModuleBuilder.cpp | 2 +- lib/Frontend/ASTConsumers.cpp | 2 +- lib/Index/CallGraph.cpp | 2 +- lib/Index/Indexer.cpp | 2 +- lib/Sema/SemaDecl.cpp | 13 +++------ lib/Sema/SemaDeclCXX.cpp | 4 +-- lib/Sema/SemaTemplateInstantiate.cpp | 4 +-- lib/Sema/SemaTemplateInstantiateDecl.cpp | 10 +++---- lib/Serialization/ASTReaderDecl.cpp | 2 +- lib/Serialization/ASTWriterDecl.cpp | 6 ++-- .../Checkers/NSErrorChecker.cpp | 2 +- lib/StaticAnalyzer/Core/CXXExprEngine.cpp | 4 +-- test/SemaCXX/deleted-function.cpp | 5 ++-- tools/libclang/CIndex.cpp | 2 +- 21 files changed, 72 insertions(+), 48 deletions(-) diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 2cf2618491..0a081e30fb 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -1514,6 +1514,16 @@ public: return hasBody(Definition); } + /// isDefined - Returns true if the function is defined at all, including + /// a deleted definition. Except for the behavior when the function is + /// deleted, behaves like hasBody. + bool isDefined(const FunctionDecl *&Definition) const; + + virtual bool isDefined() const { + const FunctionDecl* Definition; + return isDefined(Definition); + } + /// getBody - Retrieve the body (definition) of the function. The /// function body might be in any of the (re-)declarations of this /// function. The variant that accepts a FunctionDecl pointer will @@ -1531,10 +1541,17 @@ public: /// isThisDeclarationADefinition - Returns whether this specific /// declaration of the function is also a definition. This does not /// determine whether the function has been defined (e.g., in a - /// previous definition); for that information, use getBody. - /// FIXME: Should return true if function is deleted or defaulted. However, - /// CodeGenModule.cpp uses it, and I don't know if this would break it. + /// previous definition); for that information, use isDefined. Note + /// that this returns false for a defaulted function unless that function + /// has been implicitly defined (possibly as deleted). bool isThisDeclarationADefinition() const { + return IsDeleted || Body || IsLateTemplateParsed; + } + + /// doesThisDeclarationHaveABody - Returns whether this specific + /// declaration of the function has a body - that is, if it is a non- + /// deleted definition. + bool doesThisDeclarationHaveABody() const { return Body || IsLateTemplateParsed; } @@ -1617,8 +1634,10 @@ public: /// Integer(long double) = delete; // no construction from long double /// }; /// @endcode - bool isDeleted() const { return IsDeleted; } - void setDeleted(bool D = true) { IsDeleted = D; } + // If a function is deleted, its first declaration must be. + bool isDeleted() const { return getCanonicalDecl()->IsDeleted; } + bool isDeletedAsWritten() const { return IsDeleted && !IsDefaulted; } + void setDeletedAsWritten(bool D = true) { IsDeleted = D; } /// \brief Determines whether this is a function "main", which is /// the entry point into an executable program. diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 25a75b9ea0..5298745312 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -1193,7 +1193,7 @@ public: /// isUserProvided - True if it is either an implicit constructor or /// if it was defaulted or deleted on first declaration. bool isUserProvided() const { - return getCanonicalDecl()->isDeleted() || getCanonicalDecl()->isDefaulted(); + return !(isDeleted() || getCanonicalDecl()->isDefaulted()); } /// diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index a5ff664449..07823e0408 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -6100,7 +6100,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) { if (const FunctionDecl *FD = dyn_cast(D)) { // Forward declarations aren't required. - if (!FD->isThisDeclarationADefinition()) + if (!FD->doesThisDeclarationHaveABody()) return false; // Constructors and destructors are required. diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index e336dd771a..b57c6ab7d7 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1422,6 +1422,17 @@ bool FunctionDecl::hasBody(const FunctionDecl *&Definition) const { return false; } +bool FunctionDecl::isDefined(const FunctionDecl *&Definition) const { + for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I) { + if (I->IsDeleted || I->Body || I->IsLateTemplateParsed) { + Definition = I->IsDeleted ? I->getCanonicalDecl() : *I; + return true; + } + } + + return false; +} + Stmt *FunctionDecl::getBody(const FunctionDecl *&Definition) const { for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I) { if (I->Body) { @@ -1690,7 +1701,7 @@ bool FunctionDecl::isInlined() const { /// an externally visible symbol, but "extern inline" will not create an /// externally visible symbol. bool FunctionDecl::isInlineDefinitionExternallyVisible() const { - assert(isThisDeclarationADefinition() && "Must have the function definition"); + assert(doesThisDeclarationHaveABody() && "Must have the function definition"); assert(isInlined() && "Function must be inline"); ASTContext &Context = getASTContext(); diff --git a/lib/AST/DeclPrinter.cpp b/lib/AST/DeclPrinter.cpp index 2fd88d7c80..49f27234c0 100644 --- a/lib/AST/DeclPrinter.cpp +++ b/lib/AST/DeclPrinter.cpp @@ -400,7 +400,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { if (D->getNumParams()) POut << ", "; POut << "..."; } - } else if (D->isThisDeclarationADefinition() && !D->hasPrototype()) { + } else if (D->doesThisDeclarationHaveABody() && !D->hasPrototype()) { for (unsigned i = 0, e = D->getNumParams(); i != e; ++i) { if (i) Proto += ", "; @@ -518,9 +518,9 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) { if (D->isPure()) Out << " = 0"; - else if (D->isDeleted()) + else if (D->isDeletedAsWritten()) Out << " = delete"; - else if (D->isThisDeclarationADefinition()) { + else if (D->doesThisDeclarationHaveABody()) { if (!D->hasPrototype() && D->getNumParams()) { // This is a K&R function definition, so we need to print the // parameters. diff --git a/lib/AST/DumpXML.cpp b/lib/AST/DumpXML.cpp index 8bb39ba470..9bb3807c1d 100644 --- a/lib/AST/DumpXML.cpp +++ b/lib/AST/DumpXML.cpp @@ -482,7 +482,7 @@ struct XMLDumper : public XMLDeclVisitor, setFlag("trivial", D->isTrivial()); setFlag("returnzero", D->hasImplicitReturnZero()); setFlag("prototype", D->hasWrittenPrototype()); - setFlag("deleted", D->isDeleted()); + setFlag("deleted", D->isDeletedAsWritten()); if (D->getStorageClass() != SC_None) set("storage", VarDecl::getStorageClassSpecifierString(D->getStorageClass())); @@ -493,7 +493,7 @@ struct XMLDumper : public XMLDeclVisitor, for (FunctionDecl::param_iterator I = D->param_begin(), E = D->param_end(); I != E; ++I) dispatch(*I); - if (D->isThisDeclarationADefinition()) + if (D->doesThisDeclarationHaveABody()) dispatch(D->getBody()); } diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index 96aa725f56..2ccd6a782c 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -727,7 +727,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) { } // Forward declarations are emitted lazily on first use. - if (!FD->isThisDeclarationADefinition()) + if (!FD->doesThisDeclarationHaveABody()) return; } else { const VarDecl *VD = cast(Global); @@ -897,7 +897,7 @@ CodeGenModule::GetOrCreateLLVMFunction(llvm::StringRef MangledName, assert(FD->isUsed() && "Sema didn't mark implicit function as used!"); DeferredDeclsToEmit.push_back(D.getWithDecl(FD)); break; - } else if (FD->isThisDeclarationADefinition()) { + } else if (FD->doesThisDeclarationHaveABody()) { DeferredDeclsToEmit.push_back(D.getWithDecl(FD)); break; } diff --git a/lib/CodeGen/ModuleBuilder.cpp b/lib/CodeGen/ModuleBuilder.cpp index 8945028644..4a2c4abbeb 100644 --- a/lib/CodeGen/ModuleBuilder.cpp +++ b/lib/CodeGen/ModuleBuilder.cpp @@ -79,7 +79,7 @@ namespace { MEnd = D->decls_end(); M != MEnd; ++M) if (CXXMethodDecl *Method = dyn_cast(*M)) - if (Method->isThisDeclarationADefinition() && + if (Method->doesThisDeclarationHaveABody() && (Method->hasAttr() || Method->hasAttr())) Builder->EmitTopLevelDecl(Method); diff --git a/lib/Frontend/ASTConsumers.cpp b/lib/Frontend/ASTConsumers.cpp index ecd6ef442a..28d312a221 100644 --- a/lib/Frontend/ASTConsumers.cpp +++ b/lib/Frontend/ASTConsumers.cpp @@ -173,7 +173,7 @@ void DeclContextPrinter::PrintDeclContext(const DeclContext* DC, break; case Decl::Function: { const FunctionDecl* FD = cast(DC); - if (FD->isThisDeclarationADefinition()) + if (FD->doesThisDeclarationHaveABody()) Out << "[function] "; else Out << " "; diff --git a/lib/Index/CallGraph.cpp b/lib/Index/CallGraph.cpp index bf3f5a8a8d..94790b8fbc 100644 --- a/lib/Index/CallGraph.cpp +++ b/lib/Index/CallGraph.cpp @@ -74,7 +74,7 @@ void CallGraph::addTU(ASTContext& Ctx) { I != E; ++I) { if (FunctionDecl *FD = dyn_cast(*I)) { - if (FD->isThisDeclarationADefinition()) { + if (FD->doesThisDeclarationHaveABody()) { // Set caller's ASTContext. Entity Ent = Entity::get(FD, Prog); CallGraphNode *Node = getOrInsertFunction(Ent); diff --git a/lib/Index/Indexer.cpp b/lib/Index/Indexer.cpp index 7f21c4f303..ebba43c474 100644 --- a/lib/Index/Indexer.cpp +++ b/lib/Index/Indexer.cpp @@ -39,7 +39,7 @@ public: Decl *D = Ent.getDecl(TU->getASTContext()); if (FunctionDecl *FD = dyn_cast(D)) - if (FD->isThisDeclarationADefinition()) + if (FD->doesThisDeclarationHaveABody()) DefMap[Ent] = std::make_pair(FD, TU); } }; diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 0bf984d40a..46b38e165d 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -938,7 +938,7 @@ static void RemoveUsingDecls(LookupResult &R) { static bool IsDisallowedCopyOrAssign(const CXXMethodDecl *D) { // FIXME: Should check for private access too but access is set after we get // the decl here. - if (D->isThisDeclarationADefinition()) + if (D->doesThisDeclarationHaveABody()) return false; if (const CXXConstructorDecl *CD = dyn_cast(D)) @@ -973,10 +973,9 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const { return false; } - if (FD->isThisDeclarationADefinition() && + if (FD->doesThisDeclarationHaveABody() && Context.DeclMustBeEmitted(FD)) return false; - } else if (const VarDecl *VD = dyn_cast(D)) { if (!VD->isFileVarDecl() || VD->getType().isConstant(Context) || @@ -1907,10 +1906,6 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) { if (Old->isPure()) New->setPure(); - // Merge the "deleted" flag. - if (Old->isDeleted()) - New->setDeleted(); - // Merge attributes from the parameters. These can mismatch with K&R // declarations. if (New->getNumParams() == Old->getNumParams()) @@ -4723,7 +4718,7 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC, if (Redeclaration && Previous.isSingleResult()) { const FunctionDecl *Def; FunctionDecl *PrevFD = dyn_cast(Previous.getFoundDecl()); - if (PrevFD && PrevFD->hasBody(Def) && D.hasAttributes()) { + if (PrevFD && PrevFD->isDefined(Def) && D.hasAttributes()) { Diag(NewFD->getLocation(), diag::warn_attribute_precede_definition); Diag(Def->getLocation(), diag::note_previous_definition); } @@ -6118,7 +6113,7 @@ void Sema::CheckForFunctionRedefinition(FunctionDecl *FD) { // Don't complain if we're in GNU89 mode and the previous definition // was an extern inline function. const FunctionDecl *Definition; - if (FD->hasBody(Definition) && + if (FD->isDefined(Definition) && !canRedefineFunction(Definition, getLangOptions())) { if (getLangOptions().GNUMode && Definition->isInlineSpecified() && Definition->getStorageClass() == SC_Extern) diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index ca5fdd1fdd..41b09df9ca 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -2805,7 +2805,7 @@ static void CheckAbstractClassUsage(AbstractUsageInfo &Info, CXXMethodDecl *MD) { // No need to do the check on definitions, which require that // the return/param types be complete. - if (MD->isThisDeclarationADefinition()) + if (MD->doesThisDeclarationHaveABody()) return; // For safety's sake, just ignore it if we don't have type source @@ -7646,7 +7646,7 @@ void Sema::SetDeclDeleted(Decl *Dcl, SourceLocation DelLoc) { // If the declaration wasn't the first, we delete the function anyway for // recovery. } - Fn->setDeleted(); + Fn->setDeletedAsWritten(); } static void SearchForReturnInStmt(Sema &Self, Stmt *S) { diff --git a/lib/Sema/SemaTemplateInstantiate.cpp b/lib/Sema/SemaTemplateInstantiate.cpp index 3e1e735c85..03c2befbc5 100644 --- a/lib/Sema/SemaTemplateInstantiate.cpp +++ b/lib/Sema/SemaTemplateInstantiate.cpp @@ -2013,7 +2013,7 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation, SuppressNew) continue; - if (Function->hasBody()) + if (Function->isDefined()) continue; if (TSK == TSK_ExplicitInstantiationDefinition) { @@ -2023,7 +2023,7 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation, // specialization and is only an explicit instantiation definition // of members whose definition is visible at the point of // instantiation. - if (!Pattern->hasBody()) + if (!Pattern->isDefined()) continue; Function->setTemplateSpecializationKind(TSK, PointOfInstantiation); diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index f6cbe267b4..1f0bd4cac2 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1216,7 +1216,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D, D->isThisDeclarationADefinition()) { // Check for a function body. const FunctionDecl *Definition = 0; - if (Function->hasBody(Definition) && + if (Function->isDefined(Definition) && Definition->getTemplateSpecializationKind() == TSK_Undeclared) { SemaRef.Diag(Function->getLocation(), diag::err_redefinition) << Function->getDeclName(); @@ -1248,7 +1248,7 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D, default: if (const FunctionDecl *RPattern = R->getTemplateInstantiationPattern()) - if (RPattern->hasBody(RPattern)) { + if (RPattern->isDefined(RPattern)) { SemaRef.Diag(Function->getLocation(), diag::err_redefinition) << Function->getDeclName(); SemaRef.Diag(R->getLocation(), diag::note_previous_definition); @@ -2124,8 +2124,8 @@ TemplateDeclInstantiator::SubstFunctionType(FunctionDecl *D, bool TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New, FunctionDecl *Tmpl) { - if (Tmpl->isDeleted()) - New->setDeleted(); + if (Tmpl->isDeletedAsWritten()) + New->setDeletedAsWritten(); // If we are performing substituting explicitly-specified template arguments // or deduced template arguments into a function template and we reach this @@ -2300,7 +2300,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, FunctionDecl *Function, bool Recursive, bool DefinitionRequired) { - if (Function->isInvalidDecl() || Function->hasBody()) + if (Function->isInvalidDecl() || Function->isDefined()) return; // Never instantiate an explicit specialization. diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 2b82f90ae3..44b8c6a4a5 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -1385,7 +1385,7 @@ static bool isConsumerInterestedIn(Decl *D) { return Var->isFileVarDecl() && Var->isThisDeclarationADefinition() == VarDecl::Definition; if (FunctionDecl *Func = dyn_cast(D)) - return Func->isThisDeclarationADefinition(); + return Func->doesThisDeclarationHaveABody(); return isa(D) || isa(D); } diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp index dbcbadb686..8c1774a030 100644 --- a/lib/Serialization/ASTWriterDecl.cpp +++ b/lib/Serialization/ASTWriterDecl.cpp @@ -128,8 +128,8 @@ void ASTDeclWriter::Visit(Decl *D) { // have been written. We want it last because we will not read it back when // retrieving it from the AST, we'll just lazily set the offset. if (FunctionDecl *FD = dyn_cast(D)) { - Record.push_back(FD->isThisDeclarationADefinition()); - if (FD->isThisDeclarationADefinition()) + Record.push_back(FD->doesThisDeclarationHaveABody()); + if (FD->doesThisDeclarationHaveABody()) Writer.AddStmt(FD->getBody()); } } @@ -322,7 +322,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { Record.push_back(D->isPure()); Record.push_back(D->hasInheritedPrototype()); Record.push_back(D->hasWrittenPrototype()); - Record.push_back(D->isDeleted()); + Record.push_back(D->isDeletedAsWritten()); Record.push_back(D->isTrivial()); Record.push_back(D->hasImplicitReturnZero()); Writer.AddSourceLocation(D->getLocEnd(), Record); diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index 63a5917299..a51d8e0d19 100644 --- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -97,7 +97,7 @@ public: void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D, AnalysisManager &mgr, BugReporter &BR) const { - if (!D->isThisDeclarationADefinition()) + if (!D->doesThisDeclarationHaveABody()) return; if (!D->getResultType()->isVoidType()) return; diff --git a/lib/StaticAnalyzer/Core/CXXExprEngine.cpp b/lib/StaticAnalyzer/Core/CXXExprEngine.cpp index 54cbca08b9..ef7bc2016c 100644 --- a/lib/StaticAnalyzer/Core/CXXExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/CXXExprEngine.cpp @@ -132,7 +132,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *E, assert(CD); #if 0 - if (!(CD->isThisDeclarationADefinition() && AMgr.shouldInlineCall())) + if (!(CD->doesThisDeclarationHaveABody() && AMgr.shouldInlineCall())) // FIXME: invalidate the object. return; #endif @@ -246,7 +246,7 @@ void ExprEngine::VisitCXXDestructor(const CXXDestructorDecl *DD, const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - if (!(DD->isThisDeclarationADefinition() && AMgr.shouldInlineCall())) + if (!(DD->doesThisDeclarationHaveABody() && AMgr.shouldInlineCall())) return; // Create the context for 'this' region. const StackFrameContext *SFC = AMgr.getStackFrame(DD, diff --git a/test/SemaCXX/deleted-function.cpp b/test/SemaCXX/deleted-function.cpp index b3e1296004..6a8965ceb5 100644 --- a/test/SemaCXX/deleted-function.cpp +++ b/test/SemaCXX/deleted-function.cpp @@ -7,9 +7,8 @@ void fn() = delete; // expected-note {{candidate function has been explicitly de void fn2(); // expected-note {{previous declaration is here}} void fn2() = delete; // expected-error {{deleted definition must be first declaration}} -void fn3() = delete; -void fn3() { - // FIXME: This definition should be invalid. +void fn3() = delete; // expected-note {{previous definition is here}} +void fn3() { // expected-error {{redefinition}} } void ov(int) {} // expected-note {{candidate function}} diff --git a/tools/libclang/CIndex.cpp b/tools/libclang/CIndex.cpp index f17a415f44..eea078dd47 100644 --- a/tools/libclang/CIndex.cpp +++ b/tools/libclang/CIndex.cpp @@ -790,7 +790,7 @@ bool CursorVisitor::VisitFunctionDecl(FunctionDecl *ND) { // FIXME: Attributes? } - if (ND->isThisDeclarationADefinition() && !ND->isLateTemplateParsed()) { + if (ND->doesThisDeclarationHaveABody() && !ND->isLateTemplateParsed()) { if (CXXConstructorDecl *Constructor = dyn_cast(ND)) { // Find the initializers that were written in the source. llvm::SmallVector WrittenInits; -- 2.40.0