<div class="gmail_quote"><div class="gmail_quote">> Index: include/clang/Basic/DiagnosticParseKinds.td</div><div class="gmail_quote">> ===================================================================</div><div class="gmail_quote">
> --- include/clang/Basic/DiagnosticParseKinds.td<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 160664)</div><div class="gmail_quote">> +++ include/clang/Basic/DiagnosticParseKinds.td<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div>
<div class="gmail_quote">> @@ -412,14 +412,11 @@</div><div class="gmail_quote">> def err_expected_init_in_condition : Error<</div><div class="gmail_quote">> "variable declaration in condition must have an initializer">;</div>
<div class="gmail_quote">> def err_expected_init_in_condition_lparen : Error<</div><div class="gmail_quote">> - "variable declaration in condition cannot have a parenthesized initializer">;</div><div class="gmail_quote">
> -def err_extraneous_rparen_in_condition : Error<</div><div class="gmail_quote">> - "extraneous ')' after condition, expected a statement">;</div><div class="gmail_quote">> -def warn_parens_disambiguated_as_function_decl : Warning<</div>
<div class="gmail_quote">> - "parentheses were disambiguated as a function declarator">,</div><div class="gmail_quote">> - InGroup<VexingParse>;</div><div class="gmail_quote">> -def warn_dangling_else : Warning<</div>
<div class="gmail_quote">> - "add explicit braces to avoid dangling else">,</div><div class="gmail_quote">> + "variable declaration in condition cannot have a parenthesized initializer">;</div>
<div class="gmail_quote">> +def err_extraneous_rparen_in_condition : Error<</div><div class="gmail_quote">> + "extraneous ')' after condition, expected a statement">;</div><div class="gmail_quote">
> +def warn_dangling_else : Warning<</div><div class="gmail_quote">> + "add explicit braces to avoid dangling else">,</div><div class="gmail_quote"><br></div><div class="gmail_quote">I'm not sure how it's happened, but your patch is changing these lines from Unix to DOS line endings.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">> InGroup<DanglingElse>;</div><div class="gmail_quote">> def err_expected_member_or_base_name : Error<</div><div class="gmail_quote">> "expected class member or base class name">;</div>
<div class="gmail_quote">> Index: lib/Parse/ParseDecl.cpp</div><div class="gmail_quote">> ===================================================================</div><div class="gmail_quote">> --- lib/Parse/ParseDecl.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 160664)</div>
<div class="gmail_quote">> +++ lib/Parse/ParseDecl.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div class="gmail_quote">> @@ -4317,17 +4317,14 @@</div><div class="gmail_quote">
> // The paren may be part of a C++ direct initializer, eg. "int x(1);".</div><div class="gmail_quote">> // In such a case, check if we actually have a function declarator; if it</div><div class="gmail_quote">
> // is not, the declarator has been fully parsed.</div><div class="gmail_quote">> - if (getLangOpts().CPlusPlus && D.mayBeFollowedByCXXDirectInit()) {</div><div class="gmail_quote">> - // When not in file scope, warn for ambiguous function declarators, just</div>
<div class="gmail_quote">> - // in case the author intended it as a variable definition.</div><div class="gmail_quote">> - bool warnIfAmbiguous = D.getContext() != Declarator::FileContext;</div><div class="gmail_quote">
> - if (!isCXXFunctionDeclarator(warnIfAmbiguous))</div><div class="gmail_quote">> + bool IsAmbiguous = false;</div><div class="gmail_quote">> + if (getLangOpts().CPlusPlus && D.mayBeFollowedByCXXDirectInit() &&</div>
<div class="gmail_quote">> + !isCXXFunctionDeclarator(&IsAmbiguous))</div><div class="gmail_quote">> break;</div><div class="gmail_quote"><br></div><div class="gmail_quote">Please dedent this 'break;' by 2 spaces.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">> - }</div><div class="gmail_quote">> ParsedAttributes attrs(AttrFactory);</div><div class="gmail_quote">> BalancedDelimiterTracker T(*this, tok::l_paren);</div>
<div class="gmail_quote">> T.consumeOpen();</div><div class="gmail_quote">> - ParseFunctionDeclarator(D, attrs, T);</div><div class="gmail_quote">> + ParseFunctionDeclarator(D, attrs, T, IsAmbiguous);</div>
<div class="gmail_quote">> PrototypeScope.Exit();</div><div class="gmail_quote">> } else if (Tok.is(tok::l_square)) {</div><div class="gmail_quote">> ParseBracketDeclarator(D);</div><div class="gmail_quote">
> Index: lib/Sema/SemaType.cpp</div><div class="gmail_quote">> ===================================================================</div><div class="gmail_quote">> --- lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 160664)</div>
<div class="gmail_quote">> +++ lib/Sema/SemaType.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div class="gmail_quote">> @@ -2418,6 +2419,104 @@</div><div class="gmail_quote">> T = Context.getFunctionType(T, ArgTys.data(), ArgTys.size(), EPI);</div>
<div class="gmail_quote">> }</div><div class="gmail_quote">> </div><div class="gmail_quote">> + // If we see "T var();" at block scope, where T is a class type, it is</div><div class="gmail_quote">
> + // probably an attempt to initialize a variable, not a function</div><div class="gmail_quote">> + // declaration.</div><div class="gmail_quote">> + // We don't catch this case earlier, since there is no ambiguity here.</div>
<div class="gmail_quote">> + bool Pointer = false;</div><div class="gmail_quote">> + bool FunctionDeclarator = false;</div><div class="gmail_quote">> + for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {</div>
<div class="gmail_quote">> + DeclaratorChunk &Chunk = D.getTypeObject(i);</div><div class="gmail_quote">> + if (Chunk.Kind == DeclaratorChunk::Function) {</div><div class="gmail_quote">> + FunctionDeclarator = true;</div>
<div class="gmail_quote">> + break;</div><div class="gmail_quote">> + }</div><div class="gmail_quote">> + else if (Chunk.Kind == DeclaratorChunk::Pointer) {</div><div class="gmail_quote">> + Pointer = true;</div>
<div class="gmail_quote">> + break;</div><div class="gmail_quote">> + }</div><div class="gmail_quote">> + else if (Chunk.Kind == DeclaratorChunk::Paren)</div><div class="gmail_quote">> + continue;</div>
<div class="gmail_quote">> + else</div><div class="gmail_quote">> + break;</div><div class="gmail_quote">> + }</div><div class="gmail_quote"><br></div><div class="gmail_quote">Please move this into an "if (FTI.isAmbiguous)" check. We don't want to pay for this when building every function type. Also, if you can factor this out to a separate function, that would help avoid disrupting the flow of this function.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">> +</div><div class="gmail_quote">> + QualType RT = T->getAs<FunctionType>()->getResultType();</div><div class="gmail_quote">> + if (!RT->isVoidType() && (!RT->isReferenceType() || FTI.NumArgs == 1) &&</div>
<div class="gmail_quote">> + (FunctionDeclarator || Pointer) && FTI.isAmbiguous && </div><div class="gmail_quote">> + S.CurContext->isFunctionOrMethod() &&</div><div class="gmail_quote">
> + D.getFunctionDefinitionKind() == FDK_Declaration &&</div><div class="gmail_quote">> + D.getDeclSpec().getStorageClassSpecAsWritten()</div><div class="gmail_quote">> + == DeclSpec::SCS_unspecified) {</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">We should also be checking (RT->isRecordType() || FTI.NumArgs <= 1) here.</div></div><div class="gmail_quote"><br></div><div class="gmail_quote">On Wed, Jul 25, 2012 at 4:10 AM, Nikola Smiljanic <span dir="ltr"><<a href="mailto:popizdeh@gmail.com" target="_blank">popizdeh@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">> Because that could never be valid as an initializer.<br>
<br>
</div>I have no idea what I was thinking :)<br>
<br>
What I find confusing is that you're thinking in terms of when to<br>
suppress the warning and I'm looking for the opposite, conditions that<br>
need to be satisfied to emit the warning.<br>
<br>
If the return type is a reference then we only have to check that<br>
there is exactly one argument as this is the only valid way (that I<br>
know of) to initialize a reference. This suppresses the warning for<br>
zero arguments and more than one argument no matter if the return type<br>
is a class type or not.<br></blockquote><div><br></div><div>I don't follow. If the return type is a reference, then it's not a class type.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Here's a new patch, the only failing test is this piece of<br>
objective-c++ code that I don't know what to do about? This is<br>
ambiguous if we disregard the attribute, but the attribute makes the<br>
fixit incorrect.<br>
<br>
[[noreturn]]int(e)();<br>
</blockquote></div><br><div>Interesting. This would previously have warned if the parentheses around 'e' were dropped, so I don't really think this is a significant regression. I think, even with that regression, the patch is still a massive improvement; please add a FIXME to that test saying we shouldn't produce the vexing-parse warning. Eventually we should walk the attributes on the declarator, and if we find one for which a variable is not a valid subject but a function is, then we should suppress the warning.</div>