[PATCH] Refactor: Simplify boolean expressions in lib/Parse

Richard legalize at xmission.com
Mon Mar 23 19:15:04 PDT 2015


Thanks for all the comments on this.  This is exactly why I broke up these changes into one per module of related code :-).

In http://reviews.llvm.org/D8530#145602, @rsmith wrote:

> We have three cases here:
>
> - do we require a name? if so, it's a grouping paren
> - do we have a (possibly empty) list of parameters? if so, it's not a grouping paren
> - otherwise, it's a grouping paren
>
>   It doesn't make sense to combine the second and third cases together. If this code needs clarifying, one way to do it would be to factor out the second check into an appropriately-named function.


How would you feel about Extract Method `isGroupingParen` that looked like this:

  bool Parser::isGroupingParen(const Declarator &D) {
    if (!D.mayOmitIdentifier()) {
      // If this can't be an abstract-declarator, this *must* be a grouping
      // paren, because we haven't seen the identifier yet.
      return true;
    }
  
    if (Tok.is(tok::r_paren) ||             // 'int()' is a function.
        (getLangOpts().CPlusPlus && Tok.is(tok::ellipsis) &&
         NextToken().is(tok::r_paren)) ||   // C++ int(...)
        isDeclarationSpecifier() ||         // 'int(int)' is a function.
        isCXX11AttributeSpecifier()) {      // 'int([[]]int)' is a function.
      // This handles C99 6.7.5.3p11: in "typedef int X; void foo(X)", X is
      // considered to be a type, not a K&R identifier-list.
      return false;
    }
  
    // Otherwise, this is a grouping paren, e.g. 'int (*X)' or 'int(X)'.
    return true;
  }

and the call point then becomes:

  // If we haven't past the identifier yet (or where the identifier would be
  // stored, if this is an abstract declarator), then this is probably just
  // grouping parens. However, if this could be an abstract-declarator, then
  // this could also be the start of function arguments (consider 'void()').
  
  // If this is a grouping paren, handle:
  // direct-declarator: '(' declarator ')'
  // direct-declarator: '(' attributes declarator ')'
  if (isGroupingParen(D)) {


http://reviews.llvm.org/D8530

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list