From 8123a95c33b792d35c2e4992ba6e27882748fb0d Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Thu, 10 Apr 2008 02:22:51 +0000 Subject: [PATCH] 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 --- include/clang/AST/Decl.h | 1 + include/clang/AST/ExprCXX.h | 4 +- include/clang/Basic/DiagnosticKinds.def | 6 ++ lib/AST/Decl.cpp | 13 +++++ lib/AST/Expr.cpp | 3 +- lib/AST/ExprCXX.cpp | 4 +- lib/Parse/ParseDecl.cpp | 3 +- lib/Sema/SemaDecl.cpp | 16 ++++-- lib/Sema/SemaDeclCXX.cpp | 73 +++++++++++++++++++++++++ lib/Sema/SemaDeclObjC.cpp | 2 +- lib/Sema/SemaExpr.cpp | 10 +--- lib/Sema/SemaType.cpp | 7 ++- test/Sema/arg-invalid.c | 6 -- test/Sema/c89.c | 10 ++++ test/Sema/default2.cpp | 15 +++++ 15 files changed, 145 insertions(+), 28 deletions(-) delete mode 100644 test/Sema/arg-invalid.c diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index bce65f9b57..019673ab1b 100644 --- a/include/clang/AST/Decl.h +++ b/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(getType())->getResultType(); diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h index 3c99321a8a..039b16f846 100644 --- a/include/clang/AST/ExprCXX.h +++ b/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) { diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def index 611edf51f5..e69891ab1e 100644 --- a/include/clang/Basic/DiagnosticKinds.def +++ b/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, 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(this)) { + } else if (const CXXDefaultArgExpr *DefaultArg + = dyn_cast(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(Param->getDefaultArg()); + return child_iterator(); } Stmt::child_iterator CXXDefaultArgExpr::child_end() { - return reinterpret_cast(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 +{ + 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(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(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(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 TheCall(new CallExpr(Fn, Args, NumArgsPassed, + llvm::OwningPtr 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 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()) diff --git a/test/Sema/arg-invalid.c b/test/Sema/arg-invalid.c deleted file mode 100644 index 03ce00acc4..0000000000 --- a/test/Sema/arg-invalid.c +++ /dev/null @@ -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); } diff --git a/test/Sema/c89.c b/test/Sema/c89.c index 6a12f1cc56..920251535d 100644 --- a/test/Sema/c89.c +++ b/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}} */ + diff --git a/test/Sema/default2.cpp b/test/Sema/default2.cpp index 0fe04abc79..d72f5506bc 100644 --- a/test/Sema/default2.cpp +++ b/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}} +} -- 2.40.0