<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 23, 2015 at 2:38 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Richard Smith <<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> writes:<br>
> Author: rsmith<br>
> Date: Mon Feb 23 15:16:05 2015<br>
> New Revision: 230261<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=230261&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=230261&view=rev</a><br>
> Log:<br>
> Improve declaration / expression disambiguation around ptr-operators, and use<br>
> the presence of an abstract declarator with a ptr-operator as proof that a<br>
> construct cannot parse as an expression to improve diagnostics along error<br>
> recovery paths.<br>
<br>
</span>Reverted in r230274 - this is breaking llvm bootstrap.<br></blockquote><div><br></div><div>Thank you!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This, from include/llvm/ADT/DenseMapInfo.h:<br>
<br>
static unsigned getHashValue(const T *PtrVal) {<br>
return (unsigned((uintptr_t)PtrVal) >> 4) ^<br>
(unsigned((uintptr_t)PtrVal) >> 9);<br>
}<br>
<br>
now errors out, and does so fairly poorly:<br>
<br>
include/llvm/ADT/DenseMapInfo.h:44:33: error: expected ')'<br>
include/llvm/ADT/DenseMapInfo.h:44:21: note: to match this '('<br>
include/llvm/ADT/DenseMapInfo.h:44:41: error: expected ')'<br>
include/llvm/ADT/DenseMapInfo.h:44:12: note: to match this '('<br>
include/llvm/ADT/DenseMapInfo.h:45:33: error: expected ')'<br>
include/llvm/ADT/DenseMapInfo.h:45:21: note: to match this '('<br>
include/llvm/ADT/DenseMapInfo.h:45:41: error: expected ')'<br>
include/llvm/ADT/DenseMapInfo.h:45:12: note: to match this '('<br>
include/llvm/ADT/DenseMapInfo.h:45:46: error: expected expression<br>
<div class="HOEnZb"><div class="h5"><br>
> Modified:<br>
> cfe/trunk/include/clang/Parse/Parser.h<br>
> cfe/trunk/lib/Parse/ParseTentative.cpp<br>
> cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp<br>
> cfe/trunk/test/Parser/cxx-variadic-func.cpp<br>
> cfe/trunk/test/Parser/recovery.cpp<br>
> cfe/trunk/test/SemaTemplate/rdar9173693.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Parse/Parser.h<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Parse/Parser.h (original)<br>
> +++ cfe/trunk/include/clang/Parse/Parser.h Mon Feb 23 15:16:05 2015<br>
> @@ -1941,11 +1941,12 @@ private:<br>
> TPResult TryParsePtrOperatorSeq();<br>
> TPResult TryParseOperatorId();<br>
> TPResult TryParseInitDeclaratorList();<br>
> - TPResult TryParseDeclarator(bool mayBeAbstract, bool mayHaveIdentifier=true);<br>
> + TPResult TryParseDeclarator(bool MayBeAbstract, bool MayHaveIdentifier = true,<br>
> + bool VersusExpression = true);<br>
> TPResult<br>
> TryParseParameterDeclarationClause(bool *InvalidAsDeclaration = nullptr,<br>
> bool VersusTemplateArg = false);<br>
> - TPResult TryParseFunctionDeclarator();<br>
> + TPResult TryParseFunctionDeclarator(bool VersusExpression = true);<br>
> TPResult TryParseBracketDeclarator();<br>
> TPResult TryConsumeDeclarationSpecifier();<br>
><br>
><br>
> Modified: cfe/trunk/lib/Parse/ParseTentative.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Parse/ParseTentative.cpp (original)<br>
> +++ cfe/trunk/lib/Parse/ParseTentative.cpp Mon Feb 23 15:16:05 2015<br>
> @@ -284,7 +284,7 @@ Parser::TPResult Parser::TryParseSimpleD<br>
> Parser::TPResult Parser::TryParseInitDeclaratorList() {<br>
> while (1) {<br>
> // declarator<br>
> - TPResult TPR = TryParseDeclarator(false/*mayBeAbstract*/);<br>
> + TPResult TPR = TryParseDeclarator(false/*MayBeAbstract*/);<br>
> if (TPR != TPResult::Ambiguous)<br>
> return TPR;<br>
><br>
> @@ -361,7 +361,7 @@ bool Parser::isCXXConditionDeclaration()<br>
> assert(Tok.is(tok::l_paren) && "Expected '('");<br>
><br>
> // declarator<br>
> - TPR = TryParseDeclarator(false/*mayBeAbstract*/);<br>
> + TPR = TryParseDeclarator(false/*MayBeAbstract*/);<br>
><br>
> // In case of an error, let the declaration parsing code handle it.<br>
> if (TPR == TPResult::Error)<br>
> @@ -431,7 +431,7 @@ bool Parser::isCXXTypeId(TentativeCXXTyp<br>
> assert(Tok.is(tok::l_paren) && "Expected '('");<br>
><br>
> // declarator<br>
> - TPR = TryParseDeclarator(true/*mayBeAbstract*/, false/*mayHaveIdentifier*/);<br>
> + TPR = TryParseDeclarator(/*MayBeAbstract*/true, /*MayHaveIdentifier*/false);<br>
><br>
> // In case of an error, let the declaration parsing code handle it.<br>
> if (TPR == TPResult::Error)<br>
> @@ -623,6 +623,7 @@ Parser::isCXX11AttributeSpecifier(bool D<br>
> }<br>
><br>
> Parser::TPResult Parser::TryParsePtrOperatorSeq() {<br>
> + bool ConsumedAny = false;<br>
> while (true) {<br>
> if (Tok.is(tok::coloncolon) || Tok.is(tok::identifier))<br>
> if (TryAnnotateCXXScopeToken(true))<br>
> @@ -637,8 +638,9 @@ Parser::TPResult Parser::TryParsePtrOper<br>
> Tok.is(tok::kw_volatile) ||<br>
> Tok.is(tok::kw_restrict))<br>
> ConsumeToken();<br>
> + ConsumedAny = true;<br>
> } else {<br>
> - return TPResult::True;<br>
> + return ConsumedAny ? TPResult::True : TPResult::False;<br>
> }<br>
> }<br>
> }<br>
> @@ -734,7 +736,8 @@ Parser::TPResult Parser::TryParseOperato<br>
> return TPResult::Error;<br>
> AnyDeclSpecifiers = true;<br>
> }<br>
> - return TryParsePtrOperatorSeq();<br>
> + return TryParsePtrOperatorSeq() == TPResult::Error ? TPResult::Error<br>
> + : TPResult::True;<br>
> }<br>
><br>
> /// declarator:<br>
> @@ -790,13 +793,23 @@ Parser::TPResult Parser::TryParseOperato<br>
> /// '~' decltype-specifier [TODO]<br>
> /// template-id [TODO]<br>
> ///<br>
> -Parser::TPResult Parser::TryParseDeclarator(bool mayBeAbstract,<br>
> - bool mayHaveIdentifier) {<br>
> +Parser::TPResult Parser::TryParseDeclarator(bool MayBeAbstract,<br>
> + bool MayHaveIdentifier,<br>
> + bool VersusExpression) {<br>
> // declarator:<br>
> // direct-declarator<br>
> // ptr-operator declarator<br>
> - if (TryParsePtrOperatorSeq() == TPResult::Error)<br>
> - return TPResult::Error;<br>
> + {<br>
> + TPResult TPR = TryParsePtrOperatorSeq();<br>
> + if (TPR == TPResult::Error)<br>
> + return TPResult::Error;<br>
> + // After a ptr-operator, any of ')', ',', ';', and '...' indicates<br>
> + // that this cannot be an expression.<br>
> + if (VersusExpression && TPR == TPResult::True &&<br>
> + (Tok.is(tok::r_paren) || Tok.is(tok::comma) || Tok.is(tok::ellipsis) ||<br>
> + Tok.is(tok::semi)))<br>
> + return TPResult::True;<br>
> + }<br>
><br>
> // direct-declarator:<br>
> // direct-abstract-declarator:<br>
> @@ -806,7 +819,7 @@ Parser::TPResult Parser::TryParseDeclara<br>
> if ((Tok.is(tok::identifier) || Tok.is(tok::kw_operator) ||<br>
> (Tok.is(tok::annot_cxxscope) && (NextToken().is(tok::identifier) ||<br>
> NextToken().is(tok::kw_operator)))) &&<br>
> - mayHaveIdentifier) {<br>
> + MayHaveIdentifier) {<br>
> // declarator-id<br>
> if (Tok.is(tok::annot_cxxscope))<br>
> ConsumeToken();<br>
> @@ -819,14 +832,14 @@ Parser::TPResult Parser::TryParseDeclara<br>
> ConsumeToken();<br>
> } else if (Tok.is(tok::l_paren)) {<br>
> ConsumeParen();<br>
> - if (mayBeAbstract &&<br>
> + if (MayBeAbstract &&<br>
> (Tok.is(tok::r_paren) || // 'int()' is a function.<br>
> // 'int(...)' is a function.<br>
> (Tok.is(tok::ellipsis) && NextToken().is(tok::r_paren)) ||<br>
> isDeclarationSpecifier())) { // 'int(int)' is a function.<br>
> // '(' parameter-declaration-clause ')' cv-qualifier-seq[opt]<br>
> // exception-specification[opt]<br>
> - TPResult TPR = TryParseFunctionDeclarator();<br>
> + TPResult TPR = TryParseFunctionDeclarator(VersusExpression);<br>
> if (TPR != TPResult::Ambiguous)<br>
> return TPR;<br>
> } else {<br>
> @@ -842,14 +855,15 @@ Parser::TPResult Parser::TryParseDeclara<br>
> Tok.is(tok::kw___vectorcall) ||<br>
> Tok.is(tok::kw___unaligned))<br>
> return TPResult::True; // attributes indicate declaration<br>
> - TPResult TPR = TryParseDeclarator(mayBeAbstract, mayHaveIdentifier);<br>
> + TPResult TPR = TryParseDeclarator(MayBeAbstract, MayHaveIdentifier,<br>
> + VersusExpression);<br>
> if (TPR != TPResult::Ambiguous)<br>
> return TPR;<br>
> if (Tok.isNot(tok::r_paren))<br>
> return TPResult::False;<br>
> ConsumeParen();<br>
> }<br>
> - } else if (!mayBeAbstract) {<br>
> + } else if (!MayBeAbstract) {<br>
> return TPResult::False;<br>
> }<br>
><br>
> @@ -865,13 +879,13 @@ Parser::TPResult Parser::TryParseDeclara<br>
> // initializer that follows the declarator. Note that ctor-style<br>
> // initializers are not possible in contexts where abstract declarators<br>
> // are allowed.<br>
> - if (!mayBeAbstract && !isCXXFunctionDeclarator())<br>
> + if (!MayBeAbstract && !isCXXFunctionDeclarator())<br>
> break;<br>
><br>
> // direct-declarator '(' parameter-declaration-clause ')'<br>
> // cv-qualifier-seq[opt] exception-specification[opt]<br>
> ConsumeParen();<br>
> - TPR = TryParseFunctionDeclarator();<br>
> + TPR = TryParseFunctionDeclarator(VersusExpression);<br>
> } else if (Tok.is(tok::l_square)) {<br>
> // direct-declarator '[' constant-expression[opt] ']'<br>
> // direct-abstract-declarator[opt] '[' constant-expression[opt] ']'<br>
> @@ -1710,7 +1724,8 @@ Parser::TryParseParameterDeclarationClau<br>
><br>
> // declarator<br>
> // abstract-declarator[opt]<br>
> - TPR = TryParseDeclarator(true/*mayBeAbstract*/);<br>
> + TPR = TryParseDeclarator(/*MayBeAbstract*/true, /*MayHaveIdentifier*/true,<br>
> + /*VersusExpression*/!VersusTemplateArgument);<br>
> if (TPR != TPResult::Ambiguous)<br>
> return TPR;<br>
><br>
> @@ -1757,9 +1772,15 @@ Parser::TryParseParameterDeclarationClau<br>
><br>
> /// TryParseFunctionDeclarator - We parsed a '(' and we want to try to continue<br>
> /// parsing as a function declarator.<br>
> +///<br>
> /// If TryParseFunctionDeclarator fully parsed the function declarator, it will<br>
> -/// return TPResult::Ambiguous, otherwise it will return either False() or<br>
> -/// Error().<br>
> +/// return TPResult::Ambiguous.<br>
> +///<br>
> +/// If \p VersusExpression is true and this cannot be a function-style<br>
> +/// cast expression, returns TPResult::True.<br>
> +///<br>
> +/// Otherwise, returns TPResult::False if this can't be a function declarator<br>
> +/// and TPResult::Error if it can't be anything.<br>
> ///<br>
> /// '(' parameter-declaration-clause ')' cv-qualifier-seq[opt]<br>
> /// exception-specification[opt]<br>
> @@ -1767,7 +1788,7 @@ Parser::TryParseParameterDeclarationClau<br>
> /// exception-specification:<br>
> /// 'throw' '(' type-id-list[opt] ')'<br>
> ///<br>
> -Parser::TPResult Parser::TryParseFunctionDeclarator() {<br>
> +Parser::TPResult Parser::TryParseFunctionDeclarator(bool VersusExpression) {<br>
><br>
> // The '(' is already parsed.<br>
><br>
> @@ -1775,8 +1796,9 @@ Parser::TPResult Parser::TryParseFunctio<br>
> if (TPR == TPResult::Ambiguous && Tok.isNot(tok::r_paren))<br>
> TPR = TPResult::False;<br>
><br>
> - if (TPR == TPResult::False || TPR == TPResult::Error)<br>
> - return TPR;<br>
> + if (TPR != TPResult::Ambiguous)<br>
> + if (VersusExpression || TPR != TPResult::True)<br>
> + return TPR;<br>
><br>
> // Parse through the parens.<br>
> if (!SkipUntil(tok::r_paren, StopAtSemi))<br>
><br>
> Modified: cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp (original)<br>
> +++ cfe/trunk/test/Parser/cxx-ambig-init-templ.cpp Mon Feb 23 15:16:05 2015<br>
> @@ -170,6 +170,15 @@ namespace ElaboratedTypeSpecifiers {<br>
> };<br>
> }<br>
><br>
> +namespace AbstractPtrOperatorDeclarator {<br>
> + template <int, typename> struct X {<br>
> + operator int();<br>
> + };<br>
> + struct Y {<br>
> + void f(int a = X<0, int (*)()>());<br>
> + };<br>
> +}<br>
> +<br>
> namespace PR20459 {<br>
> template <typename EncTraits> struct A {<br>
> void foo(int = EncTraits::template TypeEnc<int, int>::val); // ok<br>
><br>
> Modified: cfe/trunk/test/Parser/cxx-variadic-func.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-variadic-func.cpp?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-variadic-func.cpp?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Parser/cxx-variadic-func.cpp (original)<br>
> +++ cfe/trunk/test/Parser/cxx-variadic-func.cpp Mon Feb 23 15:16:05 2015<br>
> @@ -1,8 +1,7 @@<br>
> // RUN: %clang_cc1 -fsyntax-only -verify %s<br>
><br>
> void f(...) {<br>
> - // FIXME: There's no disambiguation here; this is unambiguous.<br>
> - int g(int(...)); // expected-warning {{disambiguated}} expected-note {{paren}}<br>
> + int g(int(...)); // no warning, unambiguously a function declaration<br>
> }<br>
><br>
> void h(int n..., int m); // expected-error {{expected ')'}} expected-note {{to match}}<br>
><br>
> Modified: cfe/trunk/test/Parser/recovery.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/recovery.cpp?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/recovery.cpp?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Parser/recovery.cpp (original)<br>
> +++ cfe/trunk/test/Parser/recovery.cpp Mon Feb 23 15:16:05 2015<br>
> @@ -203,6 +203,14 @@ namespace pr15133 {<br>
> };<br>
> }<br>
><br>
> +namespace AbstractPtrOperator {<br>
> + // A ptr-operator and no name means we have a declaration and not an<br>
> + // expression.<br>
> + template<typename T> int f(int*, T::type); // expected-error {{missing 'typename'}}<br>
> + template<typename T> int f(int (T::*), T::type); // expected-error {{missing 'typename'}}<br>
> + template<typename T> int f(int (*)(), T::type); // expected-error {{missing 'typename'}}<br>
> +}<br>
> +<br>
> namespace InvalidEmptyNames {<br>
> // These shouldn't crash, the diagnostics aren't important.<br>
> struct ::, struct ::; // expected-error 2 {{expected identifier}} expected-error 2 {{declaration of anonymous struct must be a definition}} expected-warning {{declaration does not declare anything}}<br>
><br>
> Modified: cfe/trunk/test/SemaTemplate/rdar9173693.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/rdar9173693.cpp?rev=230261&r1=230260&r2=230261&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/rdar9173693.cpp?rev=230261&r1=230260&r2=230261&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaTemplate/rdar9173693.cpp (original)<br>
> +++ cfe/trunk/test/SemaTemplate/rdar9173693.cpp Mon Feb 23 15:16:05 2015<br>
> @@ -2,5 +2,5 @@<br>
><br>
> // <rdar://problem/9173693><br>
> template< bool C > struct assert { };<br>
> -template< bool > struct assert_arg_pred_impl { }; // expected-note 3 {{declared here}}<br>
> -template< typename Pred > assert<false> assert_not_arg( void (*)(Pred), typename assert_arg_pred<Pred>::type ); // expected-error 5 {{}}<br>
> +template< bool > struct assert_arg_pred_impl { }; // expected-note 2 {{declared here}}<br>
> +template< typename Pred > assert<false> assert_not_arg( void (*)(Pred), typename assert_arg_pred<Pred>::type ); // expected-error 3 {{}}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>