Several improvements from Doug Gregor related to default

argument handling.  I'll fix up the c89 (void) thing next.


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

9 files changed, 110 insertions, 21 deletions

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index afe4fcd37f..2ef31e42e5 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -385,6 +385,19 @@ void FunctionDecl::setParams(ParmVarDecl **NewParamInfo, unsigned NumParams) {
   }
 }
 
+/// getMinRequiredArguments - Returns the minimum number of arguments
+/// needed to call this function. This may be fewer than the number of
+/// function parameters, if some of the parameters have default
+/// arguments.
+unsigned FunctionDecl::getMinRequiredArguments() const {
+  unsigned NumRequiredArgs = getNumParams();
+  while (NumRequiredArgs > 0
+         && getParamDecl(NumRequiredArgs-1)->getDefaultArg())
+    --NumRequiredArgs;
+
+  return NumRequiredArgs;
+}
+
 //===----------------------------------------------------------------------===//
 // RecordDecl Implementation
 //===----------------------------------------------------------------------===//
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 0287aa0831..35bea75045 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -1021,7 +1021,8 @@ bool Expr::isNullPointerConstant(ASTContext &Ctx) const {
     // Accept ((void*)0) as a null pointer constant, as many other
     // implementations do.
     return PE->getSubExpr()->isNullPointerConstant(Ctx);
-  } else if (const CXXDefaultArgExpr *DefaultArg = dyn_cast<CXXDefaultArgExpr>(this)) {
+  } else if (const CXXDefaultArgExpr *DefaultArg 
+               = dyn_cast<CXXDefaultArgExpr>(this)) {
     // See through default argument expressions
     return DefaultArg->getExpr()->isNullPointerConstant(Ctx);
   }
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index 03faf8b8b5..323fdd67a1 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -48,8 +48,8 @@ Stmt::child_iterator CXXThrowExpr::child_end() {
 
 // CXXDefaultArgExpr
 Stmt::child_iterator CXXDefaultArgExpr::child_begin() {
-  return reinterpret_cast<Stmt**>(Param->getDefaultArg());
+  return child_iterator();
 }
 Stmt::child_iterator CXXDefaultArgExpr::child_end() {
-  return reinterpret_cast<Stmt**>(Param->getDefaultArg())+1;
+  return child_iterator();
 }
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 0eb95fa908..a7c638532d 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -1235,7 +1235,8 @@ void Parser::ParseParenDeclarator(Declarator &D) {
 /// [C++]   declaration-specifiers declarator '=' assignment-expression
 /// [GNU]   declaration-specifiers declarator attributes
 ///         declaration-specifiers abstract-declarator[opt] 
-/// [C++]   declaration-specifiers abstract-declarator[opt] '=' assignment-expression
+/// [C++]   declaration-specifiers abstract-declarator[opt] 
+///           '=' assignment-expression
 /// [GNU]   declaration-specifiers abstract-declarator[opt] attributes
 ///
 void Parser::ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D) {
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 5451ec27c0..803f527e50 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -833,17 +833,21 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl) {
   
       // Check for C99 6.7.5.3p10 - foo(void) is a non-varargs
       // function that takes no arguments, not a function that takes a
-      // single void argument.  FIXME: Is this really the right place
-      // to check for this? C++ says that the parameter list (void) is
-      // the same as an empty parameter list, whereas the parameter
-      // list (T) (with T typedef'd to void) is not. For C++, this
-      // should be handled in the parser. Check C89 and C99 standards
-      // to see what the correct behavior is.
+      // single void argument.
       if (FTI.NumArgs == 1 && !FTI.isVariadic && FTI.ArgInfo[0].Ident == 0 &&
           FTI.ArgInfo[0].Param &&
           !((ParmVarDecl*)FTI.ArgInfo[0].Param)->getType().getCVRQualifiers() &&
           ((ParmVarDecl*)FTI.ArgInfo[0].Param)->getType()->isVoidType()) {
         // empty arg list, don't push any params.
+        ParmVarDecl *Param = (ParmVarDecl*)FTI.ArgInfo[0].Param;
+
+        // In C++ and C89, the empty parameter-type-list must be
+        // spelled "void"; a typedef of void is not permitted. 
+        if (!getLangOptions().C99 &&
+            Param->getType() != Context.VoidTy) {
+          Diag(Param->getLocation(), diag::ext_param_typedef_of_void);
+        }
+
       } else {
         for (unsigned i = 0, e = FTI.NumArgs; i != e; ++i)
           Params.push_back((ParmVarDecl *)FTI.ArgInfo[i].Param);
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index e7bf7bff4b..3211e22890 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -14,11 +14,79 @@
 #include "Sema.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/AST/Type.h"
 #include "llvm/ADT/OwningPtr.h"
+#include "llvm/Support/Compiler.h"
 
 using namespace clang;
 
+//===----------------------------------------------------------------------===//
+// CheckDefaultArgumentVisitor
+//===----------------------------------------------------------------------===//
+
+/// CheckDefaultArgumentVisitor - Traverses the default argument of a
+/// parameter to determine whether it contains any ill-formed
+/// subexpressions. For example, this will diagnose the use of local
+/// variables or parameters within the default argument expression.
+class VISIBILITY_HIDDEN CheckDefaultArgumentVisitor 
+  : public StmtVisitor<CheckDefaultArgumentVisitor, bool>
+{
+  Sema *S;
+
+public:
+  explicit CheckDefaultArgumentVisitor(Sema *s) : S(s) {}
+
+  bool VisitExpr(Expr *Node);
+  bool VisitDeclRefExpr(DeclRefExpr *DRE);
+};
+
+/// VisitExpr - Visit all of the children of this expression.
+bool CheckDefaultArgumentVisitor::VisitExpr(Expr *Node) {
+  bool IsInvalid = false;
+  for (Stmt::child_iterator first = Node->child_begin(), 
+                            last = Node->child_end();
+       first != last; ++first)
+    IsInvalid |= Visit(*first);
+
+  return IsInvalid;
+}
+
+/// VisitDeclRefExpr - Visit a reference to a declaration, to
+/// determine whether this declaration can be used in the default
+/// argument expression.
+bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(DeclRefExpr *DRE) {
+  ValueDecl *Decl = DRE->getDecl();
+  if (ParmVarDecl *Param = dyn_cast<ParmVarDecl>(Decl)) {
+    // C++ [dcl.fct.default]p9
+    //   Default arguments are evaluated each time the function is
+    //   called. The order of evaluation of function arguments is
+    //   unspecified. Consequently, parameters of a function shall not
+    //   be used in default argument expressions, even if they are not
+    //   evaluated. Parameters of a function declared before a default
+    //   argument expression are in scope and can hide namespace and
+    //   class member names.
+    return S->Diag(DRE->getSourceRange().getBegin(), 
+                   diag::err_param_default_argument_references_param,
+                   Param->getName());
+  } else if (BlockVarDecl *BlockVar = dyn_cast<BlockVarDecl>(Decl)) {
+    // C++ [dcl.fct.default]p7
+    //   Local variables shall not be used in default argument
+    //   expressions.
+    return S->Diag(DRE->getSourceRange().getBegin(), 
+                   diag::err_param_default_argument_references_local,
+                   BlockVar->getName());
+  }
+
+  // FIXME: when Clang has support for member functions, "this"
+  // will also need to be diagnosted.
+
+  return false;
+}
+
+/// ActOnParamDefaultArgument - Check whether the default argument
+/// provided for a function parameter is well-formed. If so, attach it
+/// to the parameter declaration.
 void
 Sema::ActOnParamDefaultArgument(DeclTy *param, SourceLocation EqualLoc, 
                                 ExprTy *defarg) {
@@ -66,6 +134,11 @@ Sema::ActOnParamDefaultArgument(DeclTy *param, SourceLocation EqualLoc,
   //   parameter-declaration-clause, it shall not occur within a
   //   declarator or abstract-declarator of a parameter-declaration.
 
+  // Check that the default argument is well-formed
+  CheckDefaultArgumentVisitor DefaultArgChecker(this);
+  if (DefaultArgChecker.Visit(DefaultArg.get()))
+    return;
+
   // Okay: add the default argument to the parameter
   Param->setDefaultArg(DefaultArg.take());
 }
diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index a9efe6c251..8d09727dd0 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -58,7 +58,7 @@ void Sema::ObjCActOnStartOfMethodDef(Scope *FnBodyScope, DeclTy *D) {
   CreateImplicitParameter(FnBodyScope, PI.Ident, PI.IdentLoc, 
                           Context.getObjCSelType());
 
-  // Introduce all of the othe parameters into this scope  
+  // Introduce all of the other parameters into this scope.
   for (unsigned i = 0, e = MDecl->getNumParams(); i != e; ++i) {
     ParmVarDecl *PDecl = MDecl->getParamDecl(i);
     IdentifierInfo *II = PDecl->getIdentifier();
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 6a6cd25575..fa3a188332 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -596,7 +596,6 @@ ActOnCallExpr(ExprTy *fn, SourceLocation LParenLoc,
   Expr **Args = reinterpret_cast<Expr**>(args);
   assert(Fn && "no function call expression");
   FunctionDecl *FDecl = NULL;
-  unsigned NumArgsPassed = NumArgs;
 
   // Promote the function operand.
   UsualUnaryConversions(Fn);
@@ -609,9 +608,7 @@ ActOnCallExpr(ExprTy *fn, SourceLocation LParenLoc,
 
   // Make the call expr early, before semantic checks.  This guarantees cleanup
   // of arguments and function on error.
-  if (getLangOptions().CPlusPlus && FDecl && NumArgs < FDecl->getNumParams())
-    NumArgsPassed = FDecl->getNumParams();
-  llvm::OwningPtr<CallExpr> TheCall(new CallExpr(Fn, Args, NumArgsPassed,
+  llvm::OwningPtr<CallExpr> TheCall(new CallExpr(Fn, Args, NumArgs,
                                                  Context.BoolTy, RParenLoc));
   
   // C99 6.5.2.2p1 - "The expression that denotes the called function shall have
@@ -637,11 +634,10 @@ ActOnCallExpr(ExprTy *fn, SourceLocation LParenLoc,
     // If too few arguments are available (and we don't have default
     // arguments for the remaining parameters), don't make the call.
     if (NumArgs < NumArgsInProto) {
-      if (getLangOptions().CPlusPlus && 
-          FDecl &&
-          FDecl->getParamDecl(NumArgs)->getDefaultArg()) {
+      if (FDecl && NumArgs >= FDecl->getMinRequiredArguments()) {
         // Use default arguments for missing arguments
         NumArgsToCheck = NumArgsInProto;
+        TheCall->setNumArgs(NumArgsInProto);
       } else
         return Diag(RParenLoc, diag::err_typecheck_call_too_few_args,
                     Fn->getSourceRange());
diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index 4393b80a66..5e155f8817 100644
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -394,7 +394,8 @@ QualType Sema::GetTypeForDeclarator(Declarator &D, Scope *S) {
         llvm::SmallVector<QualType, 16> ArgTys;
         
         for (unsigned i = 0, e = FTI.NumArgs; i != e; ++i) {
-          QualType ArgTy = ((ParmVarDecl *)FTI.ArgInfo[i].Param)->getType();
+          ParmVarDecl *Param = (ParmVarDecl *)FTI.ArgInfo[i].Param;
+          QualType ArgTy = Param->getType();
           assert(!ArgTy.isNull() && "Couldn't parse type?");
           //
           // Perform the default function/array conversion (C99 6.7.5.3p[7,8]).
@@ -425,13 +426,13 @@ QualType Sema::GetTypeForDeclarator(Declarator &D, Scope *S) {
             if (FTI.NumArgs != 1 || FTI.isVariadic) {
               Diag(DeclType.Loc, diag::err_void_only_param);
               ArgTy = Context.IntTy;
-              ((ParmVarDecl *)FTI.ArgInfo[i].Param)->setType(ArgTy);
+              Param->setType(ArgTy);
             } else if (FTI.ArgInfo[i].Ident) {
               // Reject, but continue to parse 'int(void abc)'.
               Diag(FTI.ArgInfo[i].IdentLoc,
                    diag::err_param_with_void_type);
               ArgTy = Context.IntTy;
-              ((ParmVarDecl *)FTI.ArgInfo[i].Param)->setType(ArgTy);
+              Param->setType(ArgTy);
             } else {
               // Reject, but continue to parse 'float(const void)'.
               if (ArgTy.getCVRQualifiers())