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/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
