Hi John, Attached is a slightly modified version of the patch.
> 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. I have done this. > 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. While I originally agreed with you on this, I was at home, didn't have the patch available and couldn't see that actually the patch *does* actually do the right thing. I have added an extra testcase however as this behaviour is not currently tested. See i() in SemaCXX/default1.cpp. > Also, don't we need to look for previous declarations that *are* in the same > scope and make sure we use their default arguments? Again, this wasn't tested but does do the right thing. I've added a testcase into CodeGenCXX/default-arguments.cpp based on the snippet you posted. Is this OK now? Cheers, James -----Original Message----- From: John McCall [mailto:[email protected]] Sent: 09 March 2012 18:10 To: James Molloy Cc: [email protected] Subject: Re: [cfe-commits] [PATCH] Allow redeclaring a function in an inner scope with different default arguments 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.
cxx-inner-scope-default-args.v2.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
