[clang/18432] Fix friend decl & default arg

[Richard Smith]

Sorry for the delay. The patch looks good, other than some style issues:

+      if (OldParam->hasUnparsedDefaultArg ())
+        NewParam->setUnparsedDefaultArg ();

No spaces before ().

   if (!NeedLateParse)
     // Look ahead to see if there are any default args
-    for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)
-      if (FTI.Params[ParamIdx].DefaultArgTokens) {
+    for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx) {
+      auto Param = cast<ParmVarDecl>(FTI.Params[ParamIdx].Param);
+      if (Param->hasUnparsedDefaultArg()) {
         NeedLateParse = true;
         break;
       }
+    }

Please add braces around this 'if', since its body contains many lines.

test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p9.cpp

I'm not really sure what part of p9 this is testing; it seems more to be a
test of p4, where inheritance of default arguments is specified.

On Fri, Feb 6, 2015 at 5:33 AM, Nathan Sidwell <nathan at acm.org> wrote:

> ping?
>
>
> On 01/30/15 13:09, Nathan Sidwell wrote:
>
>> This patch fixes 18432, where a friend decl for a nested class's member
>> function
>> clobbered its default arg.  http://llvm.org/bugs/show_bug.cgi?id=18432
>>
>> As the report says, such a friend decl is unneeded, but it shouldn't break
>> things.  There are 3 components to the problem:
>>
>> 1) Sema::MergeCXXFunctionDecl doesn't realize the Older decl could have an
>> unparsed default argument.  Fixed by propagating that info to the new
>> decl (but
>> not the tokens themselves).  We now have a new possible state for a
>> ParmVarDecl
>> -- null tokens but hasUnparsedDefaultArg set.
>>
>> 2) HandleMemberFunctionDeclDelays only looked at if there were default arg
>> tokens to parse.  Changed to check the hasUnparsedDefaultArg state
>> instead. This
>> puts both the original function decl and the befriending decl on the late
>> parsing list.
>>
>> 3)  Parser::ParseLexedMethodDeclaration now needs to handle the case of
>> an
>> inherited default parm.  At the point it meets this decl, the original
>> default
>> parm will have been parsed, so it just needs to duplicate the code that
>> is now
>> skipped in MergeCXXFunctionDecl.
>>
>> It has to look at the hasUnparsedDefaultArg flag before the
>> ActOnDelayedCXXMethodParameter call, because that clears the state.  I
>> wonder
>> whether the processing I've added to ParseLexedMethodDeclaration should
>> be put
>> in ActOnDelayedCXXMethodParameter anyway?  (we'd need to add FunctionDecl
>> and
>> ParmIndex parameters if so).  Alternatively should the now duplicated code
>> propagating the default arg be broken out of MergeCXXFunctionDecl and
>> called
>> from there and ParseLexedMethodDeclaration?
>>
>> I added a new test
>> tools/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p9.cpp, as
>> this is a
>> bug in default arg handling, not in friendliness.
>>
>> tested on x86_64-linux, ok?
>>
>> nathan
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150217/2013d7d5/attachment.html>