Be sure to instantiate the parameters of a function, even when the

function's type is (strictly speaking) non-dependent. This ensures
that, e.g., default function arguments get instantiated properly.

And, since I couldn't resist, collapse the two implementations of
function-parameter instantiation into calls to a single, new function
(Sema::SubstParmVarDecl), since the two had nearly identical code (and
each had bugs the other didn't!). More importantly, factored out the
semantic analysis of a parameter declaration into
Sema::CheckParameter, which is called both by
Sema::ActOnParamDeclarator (when parameters are parsed) and when a
parameter is instantiated. Previously, we were missing some
Objective-C and address-space checks on instantiated function
parameters.

Fixes PR6733.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101029 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 126 insertions, 143 deletions

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 822e95971c..93867d7e55 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -805,6 +805,11 @@ public:
                                 bool &OverloadableAttrRequired);
   void CheckMain(FunctionDecl *FD);
   virtual DeclPtrTy ActOnParamDeclarator(Scope *S, Declarator &D);
+  ParmVarDecl *CheckParameter(DeclContext *DC, 
+                              TypeSourceInfo *TSInfo, QualType T,
+                              IdentifierInfo *Name,
+                              SourceLocation NameLoc,
+                              VarDecl::StorageClass StorageClass);
   virtual void ActOnObjCCatchParam(DeclPtrTy D);
   virtual void ActOnParamDefaultArgument(DeclPtrTy param,
                                          SourceLocation EqualLoc,
@@ -3642,7 +3647,8 @@ public:
                             const MultiLevelTemplateArgumentList &TemplateArgs,
                                         SourceLocation Loc,
                                         DeclarationName Entity);
-
+  ParmVarDecl *SubstParmVarDecl(ParmVarDecl *D, 
+                            const MultiLevelTemplateArgumentList &TemplateArgs);
   OwningExprResult SubstExpr(Expr *E,
                             const MultiLevelTemplateArgumentList &TemplateArgs);
 
@@ -3710,8 +3716,6 @@ public:
   DeclContext *FindInstantiatedContext(SourceLocation Loc, DeclContext *DC,
                           const MultiLevelTemplateArgumentList &TemplateArgs);
 
-  bool CheckInstantiatedParams(llvm::SmallVectorImpl<ParmVarDecl *> &Params);
-
   // Objective-C declarations.
   virtual DeclPtrTy ActOnStartClassInterface(SourceLocation AtInterfaceLoc,
                                              IdentifierInfo *ClassName,
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 7475ae4fbe..8e0e369af8 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4118,55 +4118,23 @@ Sema::ActOnParamDeclarator(Scope *S, Declarator &D) {
     }
   }
 
-  // Parameters can not be abstract class types.
-  // For record types, this is done by the AbstractClassUsageDiagnoser once
-  // the class has been completely parsed.
-  if (!CurContext->isRecord() &&
-      RequireNonAbstractType(D.getIdentifierLoc(), parmDeclType,
-                             diag::err_abstract_type_in_decl,
-                             AbstractParamType))
-    D.setInvalidType(true);
-
-  QualType T = adjustParameterType(parmDeclType);
-
   // Temporarily put parameter variables in the translation unit, not
   // the enclosing context.  This prevents them from accidentally
   // looking like class members in C++.
-  DeclContext *DC = Context.getTranslationUnitDecl();
-
-  ParmVarDecl *New
-    = ParmVarDecl::Create(Context, DC, D.getIdentifierLoc(), II,
-                          T, TInfo, StorageClass, 0);
+  ParmVarDecl *New = CheckParameter(Context.getTranslationUnitDecl(),
+                                    TInfo, parmDeclType, II, 
+                                    D.getIdentifierLoc(), StorageClass);
 
   if (D.isInvalidType())
-    New->setInvalidDecl();
-
-  // Parameter declarators cannot be interface types. All ObjC objects are
-  // passed by reference.
-  if (T->isObjCInterfaceType()) {
-    Diag(D.getIdentifierLoc(),
-         diag::err_object_cannot_be_passed_returned_by_value) << 1 << T;
-    New->setInvalidDecl();
-  }
-
+    New->setInvalidDecl();  
+  
   // Parameter declarators cannot be qualified (C++ [dcl.meaning]p1).
   if (D.getCXXScopeSpec().isSet()) {
     Diag(D.getIdentifierLoc(), diag::err_qualified_param_declarator)
       << D.getCXXScopeSpec().getRange();
     New->setInvalidDecl();
   }
-  
-  // ISO/IEC TR 18037 S6.7.3: "The type of an object with automatic storage 
-  // duration shall not be qualified by an address-space qualifier."
-  // Since all parameters have automatic store duration, they can not have
-  // an address space.
-  if (T.getAddressSpace() != 0) {
-    Diag(D.getIdentifierLoc(),  
-         diag::err_arg_with_address_space);
-    New->setInvalidDecl();
-  }   
-  
-  
+
   // Add the parameter declaration into this scope.
   S->AddDecl(DeclPtrTy::make(New));
   if (II)
@@ -4180,6 +4148,43 @@ Sema::ActOnParamDeclarator(Scope *S, Declarator &D) {
   return DeclPtrTy::make(New);
 }
 
+ParmVarDecl *Sema::CheckParameter(DeclContext *DC, 
+                                  TypeSourceInfo *TSInfo, QualType T,
+                                  IdentifierInfo *Name,
+                                  SourceLocation NameLoc,
+                                  VarDecl::StorageClass StorageClass) {
+  ParmVarDecl *New = ParmVarDecl::Create(Context, DC, NameLoc, Name,
+                                         adjustParameterType(T), TSInfo, 
+                                         StorageClass, 0);
+
+  // Parameters can not be abstract class types.
+  // For record types, this is done by the AbstractClassUsageDiagnoser once
+  // the class has been completely parsed.
+  if (!CurContext->isRecord() &&
+      RequireNonAbstractType(NameLoc, T, diag::err_abstract_type_in_decl,
+                             AbstractParamType))
+    New->setInvalidDecl();
+
+  // Parameter declarators cannot be interface types. All ObjC objects are
+  // passed by reference.
+  if (T->isObjCInterfaceType()) {
+    Diag(NameLoc,
+         diag::err_object_cannot_be_passed_returned_by_value) << 1 << T;
+    New->setInvalidDecl();
+  }
+
+  // ISO/IEC TR 18037 S6.7.3: "The type of an object with automatic storage 
+  // duration shall not be qualified by an address-space qualifier."
+  // Since all parameters have automatic store duration, they can not have
+  // an address space.
+  if (T.getAddressSpace() != 0) {
+    Diag(NameLoc, diag::err_arg_with_address_space);
+    New->setInvalidDecl();
+  }   
+
+  return New;
+}
+
 void Sema::ActOnObjCCatchParam(DeclPtrTy D) {
   ParmVarDecl *Param = cast<ParmVarDecl>(D.getAs<Decl>());
   Param->setDeclContext(CurContext);
diff --git a/lib/Sema/SemaTemplateInstantiate.cpp b/lib/Sema/SemaTemplateInstantiate.cpp
index a24d00f595..0e2d7f1f10 100644
--- a/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/lib/Sema/SemaTemplateInstantiate.cpp
@@ -807,47 +807,12 @@ TemplateInstantiator::TransformFunctionTypeParams(FunctionProtoTypeLoc TL,
         TransformFunctionTypeParams(TL, PTypes, PVars))
     return true;
 
-  // Check instantiated parameters.
-  if (SemaRef.CheckInstantiatedParams(PVars))
-    return true;
-
   return false;
 }
 
 ParmVarDecl *
 TemplateInstantiator::TransformFunctionTypeParam(ParmVarDecl *OldParm) {
-  TypeSourceInfo *OldDI = OldParm->getTypeSourceInfo();
-  TypeSourceInfo *NewDI = getDerived().TransformType(OldDI);
-  if (!NewDI)
-    return 0;
-
-  // TODO: do we have to clone this decl if the types match and
-  // there's no default argument?
-
-  ParmVarDecl *NewParm
-    = ParmVarDecl::Create(SemaRef.Context,
-                          OldParm->getDeclContext(),
-                          OldParm->getLocation(),
-                          OldParm->getIdentifier(),
-                          NewDI->getType(),
-                          NewDI,
-                          OldParm->getStorageClass(),
-                          /* DefArg */ NULL);
-
-  // Maybe adjust new parameter type.
-  NewParm->setType(SemaRef.adjustParameterType(NewParm->getType()));
-
-  // Mark the (new) default argument as uninstantiated (if any).
-  if (OldParm->hasUninstantiatedDefaultArg()) {
-    Expr *Arg = OldParm->getUninstantiatedDefaultArg();
-    NewParm->setUninstantiatedDefaultArg(Arg);
-  } else if (Expr *Arg = OldParm->getDefaultArg())
-    NewParm->setUninstantiatedDefaultArg(Arg);
-
-  NewParm->setHasInheritedDefaultArg(OldParm->hasInheritedDefaultArg());
-
-  SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldParm, NewParm);
-  return NewParm;
+  return SemaRef.SubstParmVarDecl(OldParm, TemplateArgs);
 }
 
 QualType
@@ -1009,6 +974,40 @@ TypeSourceInfo *Sema::SubstFunctionDeclType(TypeSourceInfo *T,
   return TLB.getTypeSourceInfo(Context, Result);
 }
 
+ParmVarDecl *Sema::SubstParmVarDecl(ParmVarDecl *OldParm, 
+                          const MultiLevelTemplateArgumentList &TemplateArgs) {
+  TypeSourceInfo *OldDI = OldParm->getTypeSourceInfo();
+  TypeSourceInfo *NewDI = SubstType(OldDI, TemplateArgs, OldParm->getLocation(),
+                                    OldParm->getDeclName());
+  if (!NewDI)
+    return 0;
+
+  if (NewDI->getType()->isVoidType()) {
+    Diag(OldParm->getLocation(), diag::err_param_with_void_type);
+    return 0;
+  }
+
+  ParmVarDecl *NewParm = CheckParameter(Context.getTranslationUnitDecl(),
+                                        NewDI, NewDI->getType(),
+                                        OldParm->getIdentifier(),
+                                        OldParm->getLocation(),
+                                        OldParm->getStorageClass());
+  if (!NewParm)
+    return 0;
+                                                
+  // Mark the (new) default argument as uninstantiated (if any).
+  if (OldParm->hasUninstantiatedDefaultArg()) {
+    Expr *Arg = OldParm->getUninstantiatedDefaultArg();
+    NewParm->setUninstantiatedDefaultArg(Arg);
+  } else if (Expr *Arg = OldParm->getDefaultArg())
+    NewParm->setUninstantiatedDefaultArg(Arg);
+
+  NewParm->setHasInheritedDefaultArg(OldParm->hasInheritedDefaultArg());
+
+  CurrentInstantiationScope->InstantiatedLocal(OldParm, NewParm);
+  return NewParm;  
+}
+
 /// \brief Perform substitution on the base class specifiers of the
 /// given class template specialization.
 ///
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 82270451cb..18b6829744 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1343,41 +1343,7 @@ Decl *TemplateDeclInstantiator::VisitCXXConversionDecl(CXXConversionDecl *D) {
 }
 
 ParmVarDecl *TemplateDeclInstantiator::VisitParmVarDecl(ParmVarDecl *D) {
-  QualType T;
-  TypeSourceInfo *DI = D->getTypeSourceInfo();
-  if (DI) {
-    DI = SemaRef.SubstType(DI, TemplateArgs, D->getLocation(),
-                           D->getDeclName());
-    if (DI) T = DI->getType();
-  } else {
-    T = SemaRef.SubstType(D->getType(), TemplateArgs, D->getLocation(),
-                          D->getDeclName());
-    DI = 0;
-  }
-
-  if (T.isNull())
-    return 0;
-
-  T = SemaRef.adjustParameterType(T);
-
-  // Allocate the parameter
-  ParmVarDecl *Param
-    = ParmVarDecl::Create(SemaRef.Context,
-                          SemaRef.Context.getTranslationUnitDecl(),
-                          D->getLocation(),
-                          D->getIdentifier(), T, DI, D->getStorageClass(), 0);
-
-  // Mark the default argument as being uninstantiated.
-  if (D->hasUninstantiatedDefaultArg())
-    Param->setUninstantiatedDefaultArg(D->getUninstantiatedDefaultArg());
-  else if (Expr *Arg = D->getDefaultArg())
-    Param->setUninstantiatedDefaultArg(Arg);
-  
-  // Note: we don't try to instantiate function parameters until after
-  // we've instantiated the function's type. Therefore, we don't have
-  // to check for 'void' parameter types here.
-  SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, Param);
-  return Param;
+  return SemaRef.SubstParmVarDecl(D, TemplateArgs);
 }
 
 Decl *TemplateDeclInstantiator::VisitTemplateTypeParmDecl(
@@ -1797,29 +1763,6 @@ TemplateDeclInstantiator::InstantiateClassTemplatePartialSpecialization(
   return false;
 }
 
-bool
-Sema::CheckInstantiatedParams(llvm::SmallVectorImpl<ParmVarDecl*> &Params) {
-  bool Invalid = false;
-  for (unsigned i = 0, i_end = Params.size(); i != i_end; ++i)
-    if (ParmVarDecl *PInst = Params[i]) {
-      if (PInst->isInvalidDecl())
-        Invalid = true;
-      else if (PInst->getType()->isVoidType()) {
-        Diag(PInst->getLocation(), diag::err_param_with_void_type);
-        PInst->setInvalidDecl();
-        Invalid = true;
-      }
-      else if (RequireNonAbstractType(PInst->getLocation(),
-                                      PInst->getType(),
-                                      diag::err_abstract_type_in_decl,
-                                      Sema::AbstractParamType)) {
-        PInst->setInvalidDecl();
-        Invalid = true;
-      }
-    }
-  return Invalid;
-}
-
 TypeSourceInfo*
 TemplateDeclInstantiator::SubstFunctionType(FunctionDecl *D,
                               llvm::SmallVectorImpl<ParmVarDecl *> &Params) {
@@ -1833,13 +1776,26 @@ TemplateDeclInstantiator::SubstFunctionType(FunctionDecl *D,
   if (!NewTInfo)
     return 0;
 
-  // Get parameters from the new type info.
-  TypeLoc NewTL = NewTInfo->getTypeLoc();
-  FunctionProtoTypeLoc *NewProtoLoc = cast<FunctionProtoTypeLoc>(&NewTL);
-  assert(NewProtoLoc && "Missing prototype?");
-  for (unsigned i = 0, i_end = NewProtoLoc->getNumArgs(); i != i_end; ++i)
-    Params.push_back(NewProtoLoc->getArg(i));
-
+  if (NewTInfo != OldTInfo) {
+    // Get parameters from the new type info.
+    TypeLoc NewTL = NewTInfo->getTypeLoc();
+    FunctionProtoTypeLoc *NewProtoLoc = cast<FunctionProtoTypeLoc>(&NewTL);
+    assert(NewProtoLoc && "Missing prototype?");
+    for (unsigned i = 0, i_end = NewProtoLoc->getNumArgs(); i != i_end; ++i)
+      Params.push_back(NewProtoLoc->getArg(i));
+  } else {
+    // The function type itself was not dependent and therefore no
+    // substitution occurred. However, we still need to instantiate
+    // the function parameters themselves.
+    TypeLoc OldTL = OldTInfo->getTypeLoc();
+    FunctionProtoTypeLoc *OldProtoLoc = cast<FunctionProtoTypeLoc>(&OldTL);
+    for (unsigned i = 0, i_end = OldProtoLoc->getNumArgs(); i != i_end; ++i) {
+      ParmVarDecl *Parm = VisitParmVarDecl(OldProtoLoc->getArg(i));
+      if (!Parm)
+        return 0;
+      Params.push_back(Parm);
+    }
+  }
   return NewTInfo;
 }
 
diff --git a/test/SemaTemplate/default-expr-arguments-2.cpp b/test/SemaTemplate/default-expr-arguments-2.cpp
new file mode 100644
index 0000000000..88cc43d644
--- /dev/null
+++ b/test/SemaTemplate/default-expr-arguments-2.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -ast-dump %s 2>&1 | FileCheck %s
+
+// This is a wacky test to ensure that we're actually instantiating
+// the default rguments of the constructor when the function type is
+// otherwise non-dependent.
+namespace PR6733 {
+  template <class T>
+  class bar {
+  public: enum { kSomeConst = 128 };
+    bar(int x = kSomeConst) {}
+  };
+  
+  // CHECK: void f()
+  void f() {
+    // CHECK: bar<int> tmp =
+    // CHECK: CXXDefaultArgExpr{{.*}}'int'
+    bar<int> tmp;
+  }
+}
diff --git a/test/SemaTemplate/instantiate-method.cpp b/test/SemaTemplate/instantiate-method.cpp
index 357ea26177..635d839b2b 100644
--- a/test/SemaTemplate/instantiate-method.cpp
+++ b/test/SemaTemplate/instantiate-method.cpp
@@ -5,7 +5,7 @@ public:
   void f(T x); // expected-error{{argument may not have 'void' type}}
   void g(T*);
 
-  static int h(T, T); // expected-error 2{{argument may not have 'void' type}}
+  static int h(T, T); // expected-error {{argument may not have 'void' type}}
 };
 
 int identity(int x) { return x; }