[cfe-commits] r45511 - in /cfe/trunk: AST/Stmt.cpp AST/StmtPrinter.cpp AST/StmtSerialization.cpp Parse/ParseObjc.cpp Parse/Parser.cpp include/clang/AST/Stmt.h include/clang/AST/StmtNodes.def include/clang/Parse/Parser.h

[Chris Lattner]

On Jan 2, 2008, at 2:54 PM, Fariborz Jahanian wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=45511&view=rev
> Log:
> New declarations/defs for Objc2's foreach-statement. This is work in  
> progress.

Nice!

Some really minor thoughts:

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/Parse/ParseObjc.cpp (original)
> +++ cfe/trunk/Parse/ParseObjc.cpp Wed Jan  2 16:54:34 2008
> @@ -491,6 +491,16 @@
>   return false;
> }
>
> +///  objc-for-collection-in: 'in'
> +///
> +bool Parser::isObjCForCollectionInKW() {

I think this method should be const.  Also, would something like  
"isTokIdentifier_in()" be a better name?  I think it would be best to  
get "Tok" and "in" in there somehow that makes it more clear what this  
does.  The fact that this is part of a "for collection" statement  
isn't really important, it just returns true if the token is an  
identifier and if it is 'in'.

>
> +  if (Tok.is(tok::identifier)) {
> +    const IdentifierInfo *II = Tok.getIdentifierInfo();
> +    return II == ObjCForCollectionInKW;
> +  }

Is this more clear than just:
   if (Tok.is(tok::identifier) &&
       Tok.getIdentifierInfo() == ObjCForCollectionInKW)
     return true;

?

> +/// ObjcForCollectionStmt - This represents Objective-c's  
> collection statement;
> +/// represented as 'for (element 'in' collection-expression)' stmt.
> +///
> +class ObjcForCollectionStmt : public Stmt {
> +  enum { ELEM, COLLECTION, BODY, END_EXPR };
> +  Stmt* SubExprs[END_EXPR]; // SubExprs[ELEM] is an expression or  
> declstmt.
> +  SourceLocation ForCollectionLoc;

A really minor nit-pick is that this should probably just be "ForLoc",  
because it's the location of the 'for' token, not the location of the  
whole statement.

Otherwise, looking great,

-Chris