r230261 - Improve declaration / expression disambiguation around ptr-operators, and use

Richard Smith richard at metafoo.co.uk
Mon Feb 23 14:51:59 PST 2015


On Mon, Feb 23, 2015 at 2:38 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> Richard Smith <richard-llvm at metafoo.co.uk> writes:
> > Author: rsmith
> > Date: Mon Feb 23 15:16:05 2015
> > New Revision: 230261
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=230261&view=rev
> > Log:
> > Improve declaration / expression disambiguation around ptr-operators,
> and use
> > the presence of an abstract declarator with a ptr-operator as proof that
> a
> > construct cannot parse as an expression to improve diagnostics along
> error
> > recovery paths.
>
> Reverted in r230274 - this is breaking llvm bootstrap.
>

Thank you!


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


More information about the cfe-commits mailing list