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 <[email protected]> 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
>>
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
