<div dir="ltr">Sorry for the delay. The patch looks good, other than some style issues:<div><br></div><div><div>+      if (OldParam->hasUnparsedDefaultArg ())</div><div>+        NewParam->setUnparsedDefaultArg ();</div></div><div><br></div><div>No spaces before ().</div><div><br></div><div><div>   if (!NeedLateParse)</div><div>     // Look ahead to see if there are any default args</div><div>-    for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx)</div><div>-      if (FTI.Params[ParamIdx].DefaultArgTokens) {</div><div>+    for (unsigned ParamIdx = 0; ParamIdx < FTI.NumParams; ++ParamIdx) {</div><div>+      auto Param = cast<ParmVarDecl>(FTI.Params[ParamIdx].Param);</div><div>+      if (Param->hasUnparsedDefaultArg()) {</div><div>         NeedLateParse = true;</div><div>         break;</div><div>       }</div><div>+    }</div></div><div><br></div><div>Please add braces around this 'if', since its body contains many lines.</div><div><br></div><div>test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p9.cpp<br></div><div><br></div><div>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.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 6, 2015 at 5:33 AM, Nathan Sidwell <span dir="ltr"><<a href="mailto:nathan@acm.org" target="_blank">nathan@acm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ping?<div class="HOEnZb"><div class="h5"><br>
<br>
On 01/30/15 13:09, Nathan Sidwell wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch fixes 18432, where a friend decl for a nested class's member function<br>
clobbered its default arg.  <a href="http://llvm.org/bugs/show_bug.cgi?id=18432" target="_blank">http://llvm.org/bugs/show_bug.<u></u>cgi?id=18432</a><br>
<br>
As the report says, such a friend decl is unneeded, but it shouldn't break<br>
things.  There are 3 components to the problem:<br>
<br>
1) Sema::MergeCXXFunctionDecl doesn't realize the Older decl could have an<br>
unparsed default argument.  Fixed by propagating that info to the new decl (but<br>
not the tokens themselves).  We now have a new possible state for a ParmVarDecl<br>
-- null tokens but hasUnparsedDefaultArg set.<br>
<br>
2) HandleMemberFunctionDeclDelays only looked at if there were default arg<br>
tokens to parse.  Changed to check the hasUnparsedDefaultArg state instead. This<br>
puts both the original function decl and the befriending decl on the late<br>
parsing list.<br>
<br>
3)  Parser::<u></u>ParseLexedMethodDeclaration now needs to handle the case of an<br>
inherited default parm.  At the point it meets this decl, the original default<br>
parm will have been parsed, so it just needs to duplicate the code that is now<br>
skipped in MergeCXXFunctionDecl.<br>
<br>
It has to look at the hasUnparsedDefaultArg flag before the<br>
ActOnDelayedCXXMethodParameter call, because that clears the state.  I wonder<br>
whether the processing I've added to ParseLexedMethodDeclaration should be put<br>
in ActOnDelayedCXXMethodParameter anyway?  (we'd need to add FunctionDecl and<br>
ParmIndex parameters if so).  Alternatively should the now duplicated code<br>
propagating the default arg be broken out of MergeCXXFunctionDecl and called<br>
from there and ParseLexedMethodDeclaration?<br>
<br>
I added a new test<br>
tools/clang/test/CXX/dcl.decl/<u></u>dcl.meaning/dcl.fct.default/<u></u>p9.cpp, as this is a<br>
bug in default arg handling, not in friendliness.<br>
<br>
tested on x86_64-linux, ok?<br>
<br>
nathan<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div>