diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 2f06ee696a..d186c6f1dc 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2493,12 +2493,14 @@ public: FullExprArg Third, SourceLocation RParenLoc, Stmt *Body); - ExprResult ActOnObjCForCollectionOperand(SourceLocation forLoc, + ExprResult CheckObjCForCollectionOperand(SourceLocation forLoc, Expr *collection); StmtResult ActOnObjCForCollectionStmt(SourceLocation ForColLoc, SourceLocation LParenLoc, - Stmt *First, Expr *Second, - SourceLocation RParenLoc, Stmt *Body); + Stmt *First, Expr *collection, + SourceLocation RParenLoc); + StmtResult FinishObjCForCollectionStmt(Stmt *ForCollection, Stmt *Body); + StmtResult ActOnCXXForRangeStmt(SourceLocation ForLoc, SourceLocation LParenLoc, Stmt *LoopVar, SourceLocation ColonLoc, Expr *Collection, diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 51285306e3..39c46234f6 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -1491,6 +1491,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { // statememt before parsing the body, in order to be able to deduce the type // of an auto-typed loop variable. StmtResult ForRangeStmt; + StmtResult ForEachStmt; + if (ForRange) { ForRangeStmt = Actions.ActOnCXXForRangeStmt(ForLoc, T.getOpenLocation(), FirstPart.take(), @@ -1502,9 +1504,10 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { // Similarly, we need to do the semantic analysis for a for-range // statement immediately in order to close over temporaries correctly. } else if (ForEach) { - if (!Collection.isInvalid()) - Collection = - Actions.ActOnObjCForCollectionOperand(ForLoc, Collection.take()); + ForEachStmt = Actions.ActOnObjCForCollectionStmt(ForLoc, T.getOpenLocation(), + FirstPart.take(), + Collection.take(), + T.getCloseLocation()); } // C99 6.8.5p5 - In C99, the body of the if statement is a scope, even if @@ -1534,11 +1537,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { return StmtError(); if (ForEach) - return Actions.ActOnObjCForCollectionStmt(ForLoc, T.getOpenLocation(), - FirstPart.take(), - Collection.take(), - T.getCloseLocation(), - Body.take()); + return Actions.FinishObjCForCollectionStmt(ForEachStmt.take(), + Body.take()); if (ForRange) return Actions.FinishCXXForRangeStmt(ForRangeStmt.take(), Body.take()); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 62cd0d0d60..31d368043e 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -1385,9 +1385,10 @@ StmtResult Sema::ActOnForEachLValueExpr(Expr *E) { } ExprResult -Sema::ActOnObjCForCollectionOperand(SourceLocation forLoc, Expr *collection) { - assert(collection); - +Sema::CheckObjCForCollectionOperand(SourceLocation forLoc, Expr *collection) { + if (!collection) + return ExprError(); + // Bail out early if we've got a type-dependent expression. if (collection->isTypeDependent()) return Owned(collection); @@ -1457,8 +1458,12 @@ Sema::ActOnObjCForCollectionOperand(SourceLocation forLoc, Expr *collection) { StmtResult Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc, SourceLocation LParenLoc, - Stmt *First, Expr *Second, - SourceLocation RParenLoc, Stmt *Body) { + Stmt *First, Expr *collection, + SourceLocation RParenLoc) { + + ExprResult CollectionExprResult = + CheckObjCForCollectionOperand(ForLoc, collection); + if (First) { QualType FirstType; if (DeclStmt *DS = dyn_cast(First)) { @@ -1486,11 +1491,15 @@ Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc, if (!FirstType->isDependentType() && !FirstType->isObjCObjectPointerType() && !FirstType->isBlockPointerType()) - Diag(ForLoc, diag::err_selector_element_type) - << FirstType << First->getSourceRange(); + return StmtError(Diag(ForLoc, diag::err_selector_element_type) + << FirstType << First->getSourceRange()); } - - return Owned(new (Context) ObjCForCollectionStmt(First, Second, Body, + + if (CollectionExprResult.isInvalid()) + return StmtError(); + + return Owned(new (Context) ObjCForCollectionStmt(First, + CollectionExprResult.take(), 0, ForLoc, RParenLoc)); } @@ -1900,6 +1909,17 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc, ColonLoc, RParenLoc)); } +/// FinishObjCForCollectionStmt - Attach the body to a objective-C foreach +/// statement. +StmtResult Sema::FinishObjCForCollectionStmt(Stmt *S, Stmt *B) { + if (!S || !B) + return StmtError(); + ObjCForCollectionStmt * ForStmt = cast(S); + + ForStmt->setBody(B); + return S; +} + /// FinishCXXForRangeStmt - Attach the body to a C++0x for-range statement. /// This is a separate step from ActOnCXXForRangeStmt because analysis of the /// body cannot be performed until after the type of the range variable is diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index ec02661218..3b41f758d9 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -1267,16 +1267,6 @@ public: return getSema().ActOnObjCAutoreleasePoolStmt(AtLoc, Body); } - /// \brief Build the collection operand to a new Objective-C fast - /// enumeration statement. - /// - /// By default, performs semantic analysis to build the new statement. - /// Subclasses may override this routine to provide different behavior. - ExprResult RebuildObjCForCollectionOperand(SourceLocation forLoc, - Expr *collection) { - return getSema().ActOnObjCForCollectionOperand(forLoc, collection); - } - /// \brief Build a new Objective-C fast enumeration statement. /// /// By default, performs semantic analysis to build the new statement. @@ -1287,11 +1277,14 @@ public: Expr *Collection, SourceLocation RParenLoc, Stmt *Body) { - return getSema().ActOnObjCForCollectionStmt(ForLoc, LParenLoc, - Element, + StmtResult ForEachStmt = getSema().ActOnObjCForCollectionStmt(ForLoc, LParenLoc, + Element, Collection, - RParenLoc, - Body); + RParenLoc); + if (ForEachStmt.isInvalid()) + return StmtError(); + + return getSema().FinishObjCForCollectionStmt(ForEachStmt.take(), Body); } /// \brief Build a new C++ exception declaration. @@ -5791,10 +5784,6 @@ TreeTransform::TransformObjCForCollectionStmt( ExprResult Collection = getDerived().TransformExpr(S->getCollection()); if (Collection.isInvalid()) return StmtError(); - Collection = getDerived().RebuildObjCForCollectionOperand(S->getForLoc(), - Collection.take()); - if (Collection.isInvalid()) - return StmtError(); // Transform the body. StmtResult Body = getDerived().TransformStmt(S->getBody()); diff --git a/test/ARCMT/checking.m b/test/ARCMT/checking.m index 215a2c9215..3ad911e10a 100644 --- a/test/ARCMT/checking.m +++ b/test/ARCMT/checking.m @@ -142,7 +142,7 @@ void rdar8861761() { - (void) noninit { self = 0; // expected-error {{cannot assign to 'self' outside of a method in the init family}} - for (id x in collection) { // expected-error {{use of undeclared identifier 'collection'}} + for (__strong id x in collection) { // expected-error {{use of undeclared identifier 'collection'}} x = 0; } }