<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 13, 2009, at 1:56 PM, Anders Johnsen wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_quote"><div class="gmail_quote"><div>Hi Eli,<br> <br></div><div class="im"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
The isExplicitOkay changes look generally okay, except that I'm not<br>
sure about changing the text of the diagnostic: it's stating something<br>
that isn't actually true in C++0x.</blockquote></div><div><br>Yeah, fixed. <br></div></div></div></blockquote><div><br></div><div>I think it could still be slightly better with, e.g.,</div><div><br></div><div><div>def err_explicit_non_function : Error<</div><div> "'explicit' can only appear on constructors%select{| and conversion functions}0">;</div></div><div><br></div><div>Then, when you emit the diagnostic, use, e.g.,</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Diag(Loc, diag::err_explicit_non_function) << getLangOptions().CPlusPlus0x;</div><div><br></div><blockquote type="cite"><div class="gmail_quote"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); margin-top: 0pt; margin-right: 0pt; margin-bottom: 0pt; margin-left: 0.8ex; padding-left: 1ex; position: static; z-index: auto; ">
I don't think the change to the signature of<br>
DiagnoseFunctionSpecifiers is a good idea: I never actually got around<br>
to implementing it, but it makes it impossible to fix the FIXME at the<br>
top of the function. It may seem like code duplication, but I think<br>
I'd prefer to use separate diagnostics for the<br>
ParsedFreeStandingDeclSpec case.</blockquote></div><div><br>Changed back, and tried to fix the FIXME. (should be removed if solution is approved)<br></div></div></div></blockquote></div><div><br></div><div>I definitely prefer it this way, where it points at the identifier. It makes much more sense for, e.g.,</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> inline int f(int), g;</span></div><div><br></div><div>I suggest that we introduce fix-it hints that remove the erroneous specifier, e.g., for inline on non-functions, use:</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Diag(D.getIdentifierLoc(), diag::err_inline_non_function)</div><div><span class="Apple-tab-span" style="white-space:pre"> </span><< CodeModificationHint::CreateRemoval(SourceRange(D.getDeclSpec().getInlineSpecLoc()))</div><div><span class="Apple-tab-span" style="white-space:pre"> </span><< SourceRange(D.getDeclSpec().getInlineSpecLoc());</div><div><br></div><div>With those tweaks, this patch looks good. Thanks!</div><div><br></div><div> - Doug</div></body></html>