[cfe-commits] [PATCH] ParseFunctionDeclarator

[Sebastian Redl]

On 04.07.2011 19:34, John Freeman wrote:
> Attached is an updated patch for ParseFunctionDeclarator. Main points:
>
> - Parameter parsing is divided between 
> ParseFunctionDeclaratorIdentifierList (existing) and 
> ParseParameterDeclarationClause (new).
> - The decision to parse as an identifier-list has been factored to a 
> new function, isFunctionDeclaratorIdentifierList.
> - In my opinion, the code structure is easier to read and better 
> resembles the actual grammar it is parsing.
> - Parameters and backtracking have been removed where possible.
> - The number of calls to DeclaratorChunk::getFunction and 
> Declarator::AddTypeInfo have been reduced from 3 to 1.
> - No tests were broken.
One tiny comment:

> @@ -3681,41 +3663,71 @@
>     // lparen is already consumed!
>     assert(D.isPastIdentifier()&&  "Should not call before identifier!");
>
> +  // This should be true when the function has at least one typed argument.
> +  // Otherwise, it is treated as a K&R-style function.
> +  bool HasProto = false;
This comment is confusing. You can't mix typed and untyped arguments, so 
"at least one typed argument" doesn't make much sense to me.

Other than that, I love this patch. That was some horrible code 
duplication that you removed.

Sebastian