<div class="gmail_quote">On Mon, Mar 26, 2012 at 6:18 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@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="HOEnZb"><div class="h5">On Mon, Mar 26, 2012 at 5:56 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Mon Mar 26 19:56:56 2012<br>
> New Revision: 153488<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=153488&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=153488&view=rev</a><br>
> Log:<br>
> When we see 'Class(X' or 'Class::Class(X' and we suspect that it names a<br>
> constructor, but X is not a known typename, check whether the tokens could<br>
> possibly match the syntax of a declarator before concluding that it isn't<br>
> a constructor. If it's definitely ill-formed, assume it is a constructor.<br>
><br>
> Empirical evidence suggests that this pattern is much more often a<br>
> constructor with a typoed (or not-yet-declared) type name than any of the<br>
> other possibilities, so the extra cost of the check is not expected to be<br>
> problematic.<br>
><br>
> Modified:<br>
>    cfe/trunk/lib/Parse/ParseDecl.cpp<br>
>    cfe/trunk/test/Parser/cxx-class.cpp<br>
>    cfe/trunk/test/SemaCXX/copy-assignment.cpp<br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=153488&r1=153487&r2=153488&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=153488&r1=153487&r2=153488&view=diff</a><br>

> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Mar 26 19:56:56 2012<br>
> @@ -3379,7 +3379,42 @@<br>
>   // Check whether the next token(s) are part of a declaration<br>
>   // specifier, in which case we have the start of a parameter and,<br>
>   // therefore, we know that this is a constructor.<br>
> -  bool IsConstructor = isDeclarationSpecifier();<br>
> +  bool IsConstructor = false;<br>
> +  if (isDeclarationSpecifier())<br>
> +    IsConstructor = true;<br>
> +  else if (Tok.is(tok::identifier) ||<br>
> +           (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::identifier))) {<br>
> +    // We've seen "C ( X" or "C ( X::Y", but "X" / "X::Y" is not a type.<br>
> +    // This might be a parenthesized member name, but is more likely to<br>
> +    // be a constructor declaration with an invalid argument type. Keep<br>
> +    // looking.<br>
> +    if (Tok.is(tok::annot_cxxscope))<br>
> +      ConsumeToken();<br>
> +    ConsumeToken();<br>
> +<br>
> +    // If this is not a constructor, we must be parsing a declarator,<br>
> +    // which must have one of the following syntactic forms:<br>
> +    switch (Tok.getKind()) {<br>
> +    case tok::l_paren:<br>
> +      // C(X   (   int));<br>
> +    case tok::l_square:<br>
> +      // C(X   [   5]);<br>
> +      // C(X   [   [attribute]]);<br>
> +    case tok::coloncolon:<br>
> +      // C(X   ::   Y);<br>
> +      // C(X   ::   *p);<br>
> +    case tok::r_paren:<br>
> +      // C(X   )<br>
> +      // Assume this isn't a constructor, rather than assuming it's a<br>
> +      // constructor with an unnamed parameter of an ill-formed type.<br>
> +      break;<br>
<br>
</div></div>I really don't like adding new lists of tokens like this... can you at<br>
least leave a note pointing here from the place which actually handles<br>
this grammar?<font color="#888888"><br></font></blockquote><div><br></div><div>Done, r153490. I'm not a huge fan of this approach either, but isConstructorDeclarator is already in the business of encoding facts about these parts of the grammar to disambiguate.</div>
<div><br></div><div>Looking more into the existing code, it appears we've already slipped up here when a previous change added grammar to ParseDeclarator but didn't update isConstructorDeclarator. We incorrectly disambiguate this valid code, because we think the ellipsis unambiguously makes this a constructor:</div>
<div><br></div><div>template<typename...T> struct S { void f(S(...args[sizeof(T)])); };</div></div>