Several improvements from Doug Gregor related to default

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

llvm-svn: 49459

clang/include/clang/AST/Decl.h

@@ -352,6 +352,7 @@ public:
     return ParamInfo[i];
   }
   void setParams(ParmVarDecl **NewParamInfo, unsigned NumParams);
+  unsigned getMinRequiredArguments() const;
 
   QualType getResultType() const { 
     return cast<FunctionType>(getType())->getResultType();

clang/include/clang/AST/ExprCXX.h

@@ -149,7 +149,9 @@ namespace clang {
     Expr *getExpr() { return Param->getDefaultArg(); }
 
     virtual SourceRange getSourceRange() const {
-      return Param->getDefaultArg()->getSourceRange();
+      // Default argument expressions have no represntation in the
+      // source, so they have an empty source range.
+      return SourceRange();
     }
 
     static bool classof(const Stmt *T) {

clang/include/clang/Basic/DiagnosticKinds.def

@@ -620,6 +620,8 @@ DIAG(err_no_matching_param, ERROR,
      "parameter named '%0' is missing")
 DIAG(ext_param_not_declared, EXTENSION,
      "parameter '%0' was not declared, defaulting to type 'int'")
+DIAG(ext_param_typedef_of_void, EXTENSION,
+     "empty parameter list defined with a typedef of 'void' is a C99 feature")
 DIAG(err_param_default_argument, ERROR,
      "C does not support default arguments")
 DIAG(err_param_default_argument_redefinition, ERROR,
@@ -628,6 +630,10 @@ DIAG(err_param_default_argument_missing, ERROR,
      "missing default argument on parameter")
 DIAG(err_param_default_argument_missing_name, ERROR,
      "missing default argument on parameter '%0'")
+DIAG(err_param_default_argument_references_param, ERROR,
+     "default argument references parameter '%0'")
+DIAG(err_param_default_argument_references_local, ERROR,
+     "default argument references local variable '%0' of enclosing function")
 DIAG(err_previous_definition, ERROR,
      "previous definition is here")
 DIAG(err_previous_use, ERROR,

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

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

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

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

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

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

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

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

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

clang/test/Sema/arg-invalid.c

@@ -1,6 +0,0 @@
-// RUN: clang %s -fsyntax-only -verify
-
-void bar (void *); 
-void f11 (z)       // expected-error {{may not have 'void' type}}
-void z; 
-{ bar (&z); }

clang/test/Sema/c89.c

@@ -47,3 +47,13 @@ typedef int sometype;
 int a(sometype, y) {return 0;}  /* expected-warning {{declaration specifier missing, defaulting to 'int'}} */
 
 
+
+
+void bar (void *); 
+void f11 (z)       /* expected-error {{may not have 'void' type}} */
+void z; 
+{ bar (&z); }
+
+typedef void T;
+void foo(T); /* expected-warning {{empty parameter list defined with a typedef of 'void' is a C99 feature}} */
+

clang/test/Sema/default2.cpp

@@ -10,3 +10,18 @@ void i()
   f(0, 1);
   f(0, 1, 2);
 }
+
+
+int f1(int i, int i, int j) { // expected-error {{redefinition of parameter 'i'}}
+  i = 17;
+  return j;
+} 
+
+int x;
+void g(int x, int y = x); // expected-error {{default argument references parameter 'x'}}
+
+void h()
+{
+   int i;
+   extern void h2(int x = sizeof(i)); // expected-error {{default argument references local variable 'i' of enclosing function}}
+}