Improve merging of function declarations. Specifically:

- When we are declaring a function in local scope, we can merge with
    a visible declaration from an outer scope if that declaration
    refers to an entity with linkage. This behavior now works in C++
    and properly ignores entities without linkage.
  - Diagnose the use of "static" on a function declaration in local
    scope.
  - Diagnose the declaration of a static function after a non-static
    declaration of the same function.
  - Propagate the storage specifier to a function declaration from a
    prior declaration (PR3425)
  - Don't name-mangle "main"

llvm-svn: 65360

clang/include/clang/AST/Decl.h

@@ -64,10 +64,7 @@ class NamedDecl : public Decl {
 
 protected:
   NamedDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName N)
-    : Decl(DK, DC, L), Name(N) {}
-  
-  NamedDecl(Kind DK, DeclContext *DC, SourceLocation L, IdentifierInfo *Id)
-    : Decl(DK, DC, L), Name(Id) {}
+    : Decl(DK, DC, L), Name(N) { }
 
 public:
   /// getIdentifier - Get the identifier that names this declaration,
@@ -614,6 +611,10 @@ public:
   bool isDeleted() const { return IsDeleted; }
   void setDeleted() { IsDeleted = true; }
 
+  /// \brief Determines whether this is a function "main", which is
+  /// the entry point into an executable program.
+  bool isMain() const;
+
   /// getPreviousDeclaration - Return the previous declaration of this
   /// function.
   const FunctionDecl *getPreviousDeclaration() const {
@@ -658,8 +659,10 @@ public:
     return getType()->getAsFunctionType()->getResultType();
   }
   StorageClass getStorageClass() const { return StorageClass(SClass); }
+  void setStorageClass(StorageClass SC) { SClass = SC; }
+
   bool isInline() const { return IsInline; }
- 
+
   /// isOverloadedOperator - Whether this function declaration
   /// represents an C++ overloaded operator, e.g., "operator+".
   bool isOverloadedOperator() const { 

clang/include/clang/Basic/DiagnosticSemaKinds.def

@@ -839,6 +839,8 @@ DIAG(err_typecheck_sclass_fscope, ERROR,
      "illegal storage class on file-scoped variable")
 DIAG(err_typecheck_sclass_func, ERROR,
      "illegal storage class on function")
+DIAG(err_static_block_func, ERROR,
+     "function declared in block scope cannot have 'static' storage class")
 DIAG(err_typecheck_address_of, ERROR,
      "address of %0 requested")
 DIAG(err_typecheck_invalid_lvalue_addrof, ERROR,

clang/lib/AST/Decl.cpp

@@ -255,6 +255,11 @@ Stmt *FunctionDecl::getBody(const FunctionDecl *&Definition) const {
   return 0;
 }
 
+bool FunctionDecl::isMain() const {
+  return getDeclContext()->getLookupContext()->isTranslationUnit() &&
+    getIdentifier() && getIdentifier()->isStr("main");
+}
+
 /// \brief Returns a value indicating whether this function
 /// corresponds to a builtin function.
 ///

clang/lib/CodeGen/Mangle.cpp

@@ -70,7 +70,7 @@ bool CXXNameMangler::mangle(const NamedDecl *D) {
     else if (Context.getSourceManager().getFileCharacteristic(FD->getLocation())
           == SrcMgr::C_ExternCSystem)
       RequiresMangling = false;
-    else if (Context.getLangOptions().CPlusPlus) {
+    else if (Context.getLangOptions().CPlusPlus && !FD->isMain()) {
       // C++ requires name mangling, unless we're in a C linkage
       // specification.
       RequiresMangling = true;

clang/lib/Sema/Sema.h

@@ -265,7 +265,8 @@ public:
                                      bool &Redeclaration);
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      QualType R, Decl *LastDeclarator,
-                                     Decl* PrevDecl, bool IsFunctionDefinition,
+                                     NamedDecl* PrevDecl, 
+                                     bool IsFunctionDefinition,
                                      bool& InvalidDecl, bool &Redeclaration);
   virtual DeclTy *ActOnParamDeclarator(Scope *S, Declarator &D);
   virtual void ActOnParamDefaultArgument(DeclTy *param, 
@@ -374,6 +375,7 @@ public:
                                 Decl *LastDecl);
   bool MergeTypeDefDecl(TypedefDecl *New, Decl *Old);
   bool MergeFunctionDecl(FunctionDecl *New, Decl *Old);
+  bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old);
   bool MergeVarDecl(VarDecl *New, Decl *Old);
   bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old);
   void CheckForFileScopedRedefinitions(Scope *S, VarDecl *VD);

clang/lib/Sema/SemaDecl.cpp

@@ -210,7 +210,7 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S) {
                      IdResolver.end(),
                      std::bind1st(std::mem_fun(&NamedDecl::declarationReplaces),
                                   FD));
-    if (Redecl != IdResolver.end()) {
+    if (Redecl != IdResolver.end() && S->isDeclScope(*Redecl)) {
       // There is already a declaration of a function on our
       // IdResolver chain. Replace it with this declaration.
       S->RemoveDecl(*Redecl);
@@ -537,6 +537,15 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD) {
   QualType OldQType = Context.getCanonicalType(Old->getType());
   QualType NewQType = Context.getCanonicalType(New->getType());
   
+  if (!isa<CXXMethodDecl>(New) && !isa<CXXMethodDecl>(Old) &&
+      New->getStorageClass() == FunctionDecl::Static &&
+      Old->getStorageClass() != FunctionDecl::Static) {
+    Diag(New->getLocation(), diag::err_static_non_static)
+      << New;
+    Diag(Old->getLocation(), PrevDiag);
+    return true;
+  }
+
   if (getLangOptions().CPlusPlus) {
     // (C++98 13.1p2):
     //   Certain function declarations cannot be overloaded:
@@ -588,16 +597,8 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD) {
     // (C++98 8.3.5p3):
     //   All declarations for a function shall agree exactly in both the
     //   return type and the parameter-type-list.
-    if (OldQType == NewQType) {
-      // We have a redeclaration.
-      MergeAttributes(New, Old);
-
-      // Merge the "deleted" flag.
-      if (Old->isDeleted())
-        New->setDeleted();
-
-      return MergeCXXFunctionDecl(New, Old);
-    } 
+    if (OldQType == NewQType)
+      return MergeCompatibleFunctionDecls(New, Old);
 
     // Fall through for conflicting redeclarations and redefinitions.
   }
@@ -639,13 +640,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD) {
 
     }
 
-    MergeAttributes(New, Old);
-
-    // Merge the "deleted" flag.
-    if (Old->isDeleted())
-      New->setDeleted();
-    
-    return false;
+    return MergeCompatibleFunctionDecls(New, Old);
   }
 
   // A function that has already been declared has been redeclared or defined
@@ -671,6 +666,38 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD) {
   return true;
 }
 
+/// \brief Completes the merge of two function declarations that are
+/// known to be compatible. 
+///
+/// This routine handles the merging of attributes and other
+/// properties of function declarations form the old declaration to
+/// the new declaration, once we know that New is in fact a
+/// redeclaration of Old.
+///
+/// \returns false
+bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) {
+  // Merge the attributes
+  MergeAttributes(New, Old);
+
+  // Merge the storage class.
+  New->setStorageClass(Old->getStorageClass());
+
+  // FIXME: need to implement inline semantics
+
+  // Merge "pure" flag.
+  if (Old->isPure())
+    New->setPure();
+
+  // Merge the "deleted" flag.
+  if (Old->isDeleted())
+    New->setDeleted();
+  
+  if (getLangOptions().CPlusPlus)
+    return MergeCXXFunctionDecl(New, Old);
+
+  return false;
+}
+
 /// Predicate for C "tentative" external object definitions (C99 6.9.2).
 static bool isTentativeDefinition(VarDecl *VD) {
   if (VD->isFileVarDecl())
@@ -1626,7 +1653,7 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 NamedDecl* 
 Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                               QualType R, Decl *LastDeclarator,
-                              Decl* PrevDecl, bool IsFunctionDefinition,
+                              NamedDecl* PrevDecl, bool IsFunctionDefinition,
                               bool& InvalidDecl, bool &Redeclaration) {
   assert(R.getTypePtr()->isFunctionType());
 
@@ -1637,12 +1664,26 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   case DeclSpec::SCS_auto:
   case DeclSpec::SCS_register:
   case DeclSpec::SCS_mutable:
-    Diag(D.getIdentifierLoc(), diag::err_typecheck_sclass_func);
+    Diag(D.getDeclSpec().getStorageClassSpecLoc(), 
+         diag::err_typecheck_sclass_func);
     InvalidDecl = true;
     break;
   case DeclSpec::SCS_unspecified: SC = FunctionDecl::None; break;
   case DeclSpec::SCS_extern:      SC = FunctionDecl::Extern; break;
-  case DeclSpec::SCS_static:      SC = FunctionDecl::Static; break;
+  case DeclSpec::SCS_static: {
+    if (DC->getLookupContext()->isFunctionOrMethod()) {
+      // C99 6.7.1p5:
+      //   The declaration of an identifier for a function that has
+      //   block scope shall have no explicit storage-class specifier
+      //   other than extern
+      // See also (C++ [dcl.stc]p4).
+      Diag(D.getDeclSpec().getStorageClassSpecLoc(), 
+           diag::err_static_block_func);
+      SC = FunctionDecl::None;
+    } else
+      SC = FunctionDecl::Static; 
+    break;
+  }
   case DeclSpec::SCS_private_extern: SC = FunctionDecl::PrivateExtern;break;
   }
 
@@ -1817,11 +1858,60 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
       CheckOverloadedOperatorDeclaration(NewFD))
     NewFD->setInvalidDecl();
     
-  // Merge the decl with the existing one if appropriate. Since C functions
-  // are in a flat namespace, make sure we consider decls in outer scopes.
+  if (PrevDecl && !isDeclInScope(PrevDecl, DC, S)) {
+    // Name lookup has found a previous declaration that is not in the
+    // same scope as the new declaration. However, these two
+    // declarations might still declare the same thing (C99 6.2.2p3-4,
+    // C++ [basic.link]p6).
+
+    // FIXME: PrevDecl could be an OverloadedFunctionDecl, in which
+    // case we need to check each of the overloaded functions.
+
+    if (getLangOptions().CPlusPlus) {
+      // C++ [basic.link]p6:
+      //   If there is a visible declaration of an entity with linkage
+      //   having the same name and type, ignoring entities declared
+      //   outside the innermost enclosing namespace scope, the block
+      //   scope declaration declares that same entity and receives the
+      //   linkage of the previous declaration.
+      DeclContext *OuterContext = DC->getLookupContext();
+      if (!OuterContext->isFunctionOrMethod())
+        // This rule only applies to block-scope declarations.
+        PrevDecl = 0;
+      else {
+        DeclContext *PrevOuterContext = PrevDecl->getDeclContext();
+        if (PrevOuterContext->isRecord())
+          // We found a member function: ignore it.
+          PrevDecl = 0;
+        else {
+          // Find the innermost enclosing namespace for the new and
+          // previous declarations.
+          while (!OuterContext->isFileContext())
+            OuterContext = OuterContext->getParent();
+          while (!PrevOuterContext->isFileContext())
+            PrevOuterContext = PrevOuterContext->getParent();
+          
+          // The previous declaration is in a different namespace, so it
+          // isn't the same function.
+          if (OuterContext->getPrimaryContext() != 
+              PrevOuterContext->getPrimaryContext())
+            PrevDecl = 0;
+        }
+      }
+    }
+
+    // If the declaration we've found has no linkage, ignore it. 
+    if (VarDecl *VD = dyn_cast_or_null<VarDecl>(PrevDecl)) {
+      if (!VD->hasGlobalStorage())
+        PrevDecl = 0;
+    } else if (PrevDecl && !isa<FunctionDecl>(PrevDecl))
+      PrevDecl = 0;
+  }
+
+  // Merge or overload the declaration with an existing declaration of
+  // the same name, if appropriate.
   bool OverloadableAttrRequired = false;
-  if (PrevDecl &&
-      (!getLangOptions().CPlusPlus||isDeclInScope(PrevDecl, DC, S))) {
+  if (PrevDecl) {
     // Determine whether NewFD is an overload of PrevDecl or
     // a declaration that requires merging. If it's an overload,
     // there's no more work to do here; we'll just add the new

clang/test/CodeGen/linkage-redecl.c

@@ -0,0 +1,11 @@
+// RUN: clang -emit-llvm %s -o - |grep internal
+
+// C99 6.2.2p3
+// PR3425
+static void f(int x);
+
+void g0() {
+  f(5);
+}
+
+extern void f(int x) { } // still has internal linkage

clang/test/Sema/function-redecl.c

@@ -28,3 +28,33 @@ INT g2(x) // expected-error{{conflicting types for 'g2'}}
 {
   return x;
 }
+
+void test() {
+  int f1;
+  {
+    void f1(double);
+    {
+      void f1(double); // expected-note{{previous declaration is here}}
+      {
+        int f1(int); // expected-error{{conflicting types for 'f1'}}
+      }
+    }
+  }
+}
+
+extern void g3(int); // expected-note{{previous declaration is here}}
+static void g3(int x) { } // expected-error{{static declaration of 'g3' follows non-static declaration}}
+
+void test2() {
+  extern int f2; // expected-note{{previous definition is here}}
+  {
+    void f2(int); // expected-error{{redefinition of 'f2' as different kind of symbol}}
+  }
+
+  {
+    int f2;
+    {
+      void f2(int); // okay
+    }
+  }
+}

clang/test/Sema/function.c

@@ -53,3 +53,8 @@ void f0_3137() {
 void f1_3137() {
   int (*fp)(void) = g0_3137;
 }
+
+void f1static() {
+  static void f2static(int); // expected-error{{function declared in block scope cannot have 'static' storage class}}
+  register void f2register(int); // expected-error{{illegal storage class on function}}
+}

clang/test/Sema/nested-redef.c

@@ -17,7 +17,7 @@ struct Z {
 
 void f2(void) {
   struct T t;
-  // FIXME: this is well-formed, but Clang breaks on it struct U u;
+  struct U u;
 }
 
 

clang/test/SemaCXX/function-redecl.cpp

@@ -0,0 +1,26 @@
+// RUN: clang -fsyntax-only -verify %s
+int foo(int);
+
+namespace N {
+  void f1() {
+    void foo(int); // okay
+  }
+
+  // FIXME: we shouldn't even need this declaration to detect errors
+  // below.
+  void foo(int); // expected-note{{previous declaration is here}}
+
+  void f2() {
+    int foo(int); // expected-error{{functions that differ only in their return type cannot be overloaded}}
+
+    {
+      int foo;
+      {
+        // FIXME: should diagnose this because it's incompatible with
+        // N::foo. However, name lookup isn't properly "skipping" the
+        // "int foo" above.
+        float foo(int); 
+      }
+    }
+  }
+}