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.

llvm-svn: 131013

clang/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.

clang/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());
   }
   
   ///

clang/lib/AST/ASTContext.cpp

@@ -6100,7 +6100,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
 
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
     // Forward declarations aren't required.
-    if (!FD->isThisDeclarationADefinition())
+    if (!FD->doesThisDeclarationHaveABody())
       return false;
 
     // Constructors and destructors are required.

clang/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();
   

clang/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.

clang/lib/AST/DumpXML.cpp

@@ -482,7 +482,7 @@ struct XMLDumper : public XMLDeclVisitor<XMLDumper>,
     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<XMLDumper>,
     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());
   }
 

clang/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<VarDecl>(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;
         }

clang/lib/CodeGen/ModuleBuilder.cpp

@@ -79,7 +79,7 @@ namespace {
                                      MEnd = D->decls_end();
              M != MEnd; ++M)
           if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(*M))
-            if (Method->isThisDeclarationADefinition() &&
+            if (Method->doesThisDeclarationHaveABody() &&
                 (Method->hasAttr<UsedAttr>() || 
                  Method->hasAttr<ConstructorAttr>()))
               Builder->EmitTopLevelDecl(Method);

clang/lib/Frontend/ASTConsumers.cpp

@@ -173,7 +173,7 @@ void DeclContextPrinter::PrintDeclContext(const DeclContext* DC,
     break;
   case Decl::Function: {
     const FunctionDecl* FD = cast<FunctionDecl>(DC);
-    if (FD->isThisDeclarationADefinition())
+    if (FD->doesThisDeclarationHaveABody())
       Out << "[function] ";
     else
       Out << "<function> ";

clang/lib/Index/CallGraph.cpp

@@ -74,7 +74,7 @@ void CallGraph::addTU(ASTContext& Ctx) {
        I != E; ++I) {
 
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(*I)) {
-      if (FD->isThisDeclarationADefinition()) {
+      if (FD->doesThisDeclarationHaveABody()) {
         // Set caller's ASTContext.
         Entity Ent = Entity::get(FD, Prog);
         CallGraphNode *Node = getOrInsertFunction(Ent);

clang/lib/Index/Indexer.cpp

@@ -39,7 +39,7 @@ public:
 
     Decl *D = Ent.getDecl(TU->getASTContext());
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
-      if (FD->isThisDeclarationADefinition())
+      if (FD->doesThisDeclarationHaveABody())
         DefMap[Ent] = std::make_pair(FD, TU);
   }
 };

clang/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<CXXConstructorDecl>(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<VarDecl>(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<FunctionDecl>(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)

clang/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) {

clang/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);

clang/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.

clang/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<FunctionDecl>(D))
-    return Func->isThisDeclarationADefinition();
+    return Func->doesThisDeclarationHaveABody();
   return isa<ObjCProtocolDecl>(D) || isa<ObjCImplementationDecl>(D);
 }
 

clang/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<FunctionDecl>(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);

clang/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;

clang/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,

clang/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}}

clang/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<CXXConstructorDecl>(ND)) {
       // Find the initializers that were written in the source.
       llvm::SmallVector<CXXCtorInitializer *, 4> WrittenInits;