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

[Chris Lattner]

On Jan 29, 2008, at 10:21 AM, Fariborz Jahanian wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=46520&view=rev
> Log:
> Bunch of type defs, etc. for @synchronized.

Hey Fariborz,

> +++ cfe/trunk/AST/StmtPrinter.cpp Tue Jan 29 12:21:32 2008
> @@ -455,6 +455,17 @@
>   OS << ";\n";
> }

A couple minor things here:

> +void StmtPrinter::VisitObjCAtSynchronizedStmt  
> (ObjCAtSynchronizedStmt *Node) {

No space before the (.

>
> +  Indent() << "@synchronized (";
> +  PrintExpr(Node->getSynchExpr());
> +  OS << ")";
> +  if (CompoundStmt *CS = dyn_cast<CompoundStmt>(Node- 
> >getSynchBody()))
> +    {

Please put the { on the same line as the if and unindent the body one  
level.

>
> +      PrintRawCompoundStmt(CS);
> +      OS << "\n";
> +    }
> +}

Finally, most importantly, is it possible to have an @synchronized  
block with a non-compound-stmt body?

If so, you need to handle this in the printer.  If not, getSynchBody  
should return a CompoundStmt*, and the dyn_cast should go away.

> ++ cfe/trunk/include/clang/AST/Stmt.h Tue Jan 29 12:21:32 2008
> @@ -962,6 +962,40 @@
>   static ObjCAtTryStmt* CreateImpl(llvm::Deserializer& D);
> };
>
> +/// ObjCAtSynchronizedStmt - This is for objective-c's  
> @synchronized statement.
> +///
> +class ObjCAtSynchronizedStmt : public Stmt {

Please add a short example to the class comment.

> +private:
> +  Expr* SynchExpr;
> +  Stmt* SynchBody;
> +  SourceLocation AtSynchronizedLoc;
> +
> +public:
> +  ObjCAtSynchronizedStmt(SourceLocation atSynchronizedLoc, Expr  
> *synchExpr,
> +                         Stmt *synchBody)
> +  : Stmt(ObjCAtSynchronizedStmtClass),
> +    SynchExpr(synchExpr), SynchBody(synchBody),
> +    AtSynchronizedLoc(atSynchronizedLoc) {}
> +
> +  const Stmt *getSynchBody() const { return SynchBody; }
> +  Stmt *getSynchBody() { return SynchBody; }

If it makes sense, please m ake these be CompoundStmt's.

Thanks!

-Chris