[cfe-dev] C++ default arguments patch, rework Parse-Sema interaction for function parameters

[Doug Gregor]

Hi Chris,

On Wed, Apr 9, 2008 at 10:42 PM, Chris Lattner <clattner at apple.com> wrote:
>  +++ lib/Sema/SemaDeclCXX.cpp    (working copy)
>
>  +/// 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
>
>  Please wrap this class in an anonymous namespace.  Is there a spec citation
> that you can give for this?  Maybe C++ [dcl.fct.default] is enough.

It checks semantics for a couple different paragraphs in
[dcl.fct.default], so I've used that.

>
>  +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;
>  +}
>
>  Does it make sense to report multiple diagnostics for a malformed default?

Yes, I think it does make sense to report multiple diagnostics,
because they are all independent problems.

>  +    //   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());
>
>  It's a very minor thing, but it would be nice to highlight the sourcerange
> for the entire default arg expr as well, giving something like:
>
>  void foo(int i, int j, int k = i+j)
>                                ^~~
>
>  This would require capturing the root of the default arg expr in the
> CheckDefaultArgumentVisitor, but that doesn't seem too evil :)

Okay; that's not evil at all. Actually, it looks pretty nice.

>  +    // 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());
>
>  Note that this will also exclude local static variables and local extern
> variables.  For example:
>
>  void foo() {
>   extern int x;
>   void bar(int z = x);
>  }
>
>  Is that expected?

Yes, that's expected.

Cleanup patch attached.

  - Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-default-arg-fixes.patch
Type: text/x-patch
Size: 8925 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20080412/8ddaa5ef/attachment.bin>