Don't swap function decls, and add them to the scope as they are

encountered.  Mixing up the decls is unintuitive, and confuses the AST 
destruction code. Fixes PR2360.

Note that there is a need to look up the characteristics and 
declarations of a function associated with a particular name or decl, 
but the original swapping code doesn't solve it properly. 
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-May/001644.html is one 
suggestion for how to fix that.

llvm-svn: 51584

clang/include/clang/AST/Decl.h

@@ -446,6 +446,10 @@ public:
     return PreviousDeclaration;
   }
 
+  void setPreviousDeclaration(FunctionDecl * PrevDecl) {
+    PreviousDeclaration = PrevDecl;
+  }
+
   // Iterator access to formal parameters.
   unsigned param_size() const { return getNumParams(); }
   typedef ParmVarDecl **param_iterator;
@@ -492,10 +496,6 @@ public:
   }
   StorageClass getStorageClass() const { return StorageClass(SClass); }
   bool isInline() const { return IsInline; }
-   
-  /// AddRedeclaration - Adds the function declaration FD as a
-  /// redeclaration of this function.
-  void AddRedeclaration(FunctionDecl *FD);
  
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return D->getKind() == Function; }

clang/lib/AST/Decl.cpp

@@ -502,63 +502,6 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
   return NumRequiredArgs;
 }
 
-/// AddRedeclaration - Specifies that this function declaration has been
-/// redeclared by the function declaration FD. FD must be a
-/// redeclaration of this based on the semantics of the language being
-/// translated ("compatible" function types in C, same signatures in
-/// C++). 
-void FunctionDecl::AddRedeclaration(FunctionDecl *FD) {
-  assert(FD->PreviousDeclaration == 0 && 
-         "Redeclaration already has a previous declaration!");
-
-  // Insert FD into the list of previous declarations of this
-  // function.
-  FD->PreviousDeclaration = this->PreviousDeclaration;
-  this->PreviousDeclaration = FD;
-
-  // Swap the contents of this function declaration and FD. This
-  // effectively transforms the original declaration into the most
-  // recent declaration, so that all references to this declaration
-  // remain valid (and have information from *all* declarations),
-  // while retaining all of the information about previous
-  // declarations as well.
-
-  // Swap parameters, so that the most recent parameter names and
-  // exact types (e.g., enum vs int) show up in the original
-  // declaration.
-  std::swap(this->ParamInfo, FD->ParamInfo);
-  
-  // Swap the function body: all declarations share the same function
-  // body, but we keep track of who actually defined that function
-  // body by keeping the pointer to the body stored in that node.
-  std::swap(this->Body, FD->Body);
-
-  // Swap type information: this is important because in C, later
-  // declarations can provide slightly different types (enum vs. int,
-  // for example).
-  QualType thisType = this->getType();
-  this->setType(FD->getType());
-  FD->setType(thisType);
-
-  // Swap location information: this allows us to produce diagnostics
-  // later on that reference the most recent declaration (which has
-  // the most information!) while retaining the location of previous
-  // declarations (good for "redefinition" diagnostics).
-  SourceLocation thisLocation = this->getLocation();
-  this->setLocation(FD->getLocation());
-  FD->setLocation(thisLocation);
-  
-  // Swap attributes. FD will have the union of the attributes from
-  // all previous declarations.
-  this->swapAttrs(FD);
-
-  // If any declaration is inline, the function is inline.
-  this->IsInline |= FD->IsInline;
-
-  // FIXME: Is this the right way to handle storage specifiers?
-  if (FD->SClass) this->SClass = FD->SClass;
-}
-
 //===----------------------------------------------------------------------===//
 // RecordDecl Implementation
 //===----------------------------------------------------------------------===//

clang/lib/Sema/SemaDecl.cpp

@@ -910,13 +910,7 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl) {
       NewFD = MergeFunctionDecl(NewFD, PrevDecl, Redeclaration);
       if (NewFD == 0) return 0;
       if (Redeclaration) {
-        // Note that the new declaration is a redeclaration of the
-        // older declaration. Then return the older declaration: the
-        // new one is only kept within the set of previous
-        // declarations for this function.
-        FunctionDecl *OldFD = (FunctionDecl *)PrevDecl;
-        OldFD->AddRedeclaration(NewFD);
-        return OldFD;
+        NewFD->setPreviousDeclaration(cast<FunctionDecl>(PrevDecl));
       }
     }
     New = NewFD;

clang/test/CodeGen/func-decl-cleanup.c

@@ -0,0 +1,12 @@
+// RUN: clang %s -emit-llvm -o -
+
+
+// PR2360
+typedef void fn_t();
+
+fn_t a,b;
+
+void b()
+{
+}
+