[cfe-commits] [PATCH] Allow redeclaring a function in an inner scope with different default arguments

[John McCall]

On Mar 9, 2012, at 4:02 AM, James Molloy wrote:
> The following code is legal according to the c++ standard:
> 
> void f() {
>  int g(int a, int b=4);
>  {
>    int g(int a, int b=5);
>  }
> }
> 
> Because "[snip] That is, declarations in inner scopes do not acquire default
> arguments from declarations in outer scopes, and vice versa."
> 
> Clang currently quotes this part of the standard in a comment, but does not
> adhere to it. The attached patch fixes this so the above code compiles
> without errors.
> 
> This involves passing information about whether the new and old declarations
> are in the same Scope when merging them.

+      bool IsInSameCXXScope = true;
+      if (getLangOptions().CPlusPlus && S)
+        IsInSameCXXScope = isDeclInScope(OldDecl, NewFD->getDeclContext(), S);
+

I would prefer to only compute this when there's actually a default argument
on the old declaration.  It should be easy enough to just pass down the scope
of the new declaration.

-    if (OldParam->hasDefaultArg() && NewParam->hasDefaultArg()) {
+    bool OldParamHasDfl = OldParam->hasDefaultArg();
+    bool NewParamHasDfl = NewParam->hasDefaultArg();
+    
+    if (!IsInSameCXXScope)
+      // Ignore default parameters of old decl if they are not in
+      // the same scope.
+      OldParamHasDfl = false;

This doesn't seem to be right.  We should be ignoring the default argument
in other scopes regardless of whether the new declaration has a default
argument.

Also, don't we need to look for previous declarations that *are* in the same
scope and make sure we use their default arguments?  I'm thinking of
something like:

  void test() {
    void foo(int, int = 7);
    { void foo(int, int = 5); }
    void foo(int = 5, int); // valid, with the second argument defaulting to 7
  }

John.