Hi Richard,
I made the changes you suggested and am resubmitting it. Thanks for your
help.
- jim
On 10/5/2011 8:57 AM, Richard Smith wrote:
Hi Jim,
Functionally, the patch looks good to me. I have a concern about a similar
issue with TreeTransform (which also reuses AST nodes where possible), but it
looks like we don't currently use it in a way where it could produce the same
AST node twice within the same function.
Some small issues, then I think this is fine to check in:
Index: test/Analysis/misc-ps-cxx0x.cpp
===================================================================
--- test/Analysis/misc-ps-cxx0x.cpp (revision 141126)
+++ test/Analysis/misc-ps-cxx0x.cpp (working copy)
@@ -9,3 +9,27 @@
*p = 0xDEADBEEF; // expected-warning {{null}}
}
+// Test for correct handling of C++ ForRange statement.
+void test1() {
+ int array[2] = { 1, 2 };
+ int j = 0;
+ for ( int i : array )
+ j += i;
+
+ int *p = 0;
+ *p = 0xDEADBEEF; // expected-warning {{null}}
+}
+
+void test2() {
+ int array[2] = { 1, 2 };
+ int j = 0;
+ for (int i : array)
+ j += i;
+
+ if (j == 3)
+ return;
+
+ int *p = 0;
+ *p = 0xDEADBEEF; // no-warning
+}
+
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 141174)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -1337,12 +1337,19 @@
if (!BeginEndDecl.get() && !RangeVarType->isDependentType()) {
SourceLocation RangeLoc = RangeVar->getLocation();
- ExprResult RangeRef = BuildDeclRefExpr(RangeVar,
-
RangeVarType.getNonReferenceType(),
- VK_LValue, ColonLoc);
- if (RangeRef.isInvalid())
+ const QualType RangeVarNonRefType = RangeVarType.getNonReferenceType();
+
+ ExprResult BeginRangeRef = BuildDeclRefExpr(RangeVar,
RangeVarNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRangeRef.isInvalid())
return StmtError();
+ ExprResult EndRangeRef = BuildDeclRefExpr(RangeVar, RangeVarNonRefType,
+ VK_LValue, ColonLoc);
+
+ if (EndRangeRef.isInvalid())
+ return StmtError();
+
QualType AutoType = Context.getAutoDeductType();
Expr *Range = RangeVar->getInit();
if (!Range)
@@ -1368,8 +1375,8 @@
// the program is ill-formed;
// begin-expr is __range.
- BeginExpr = RangeRef;
- if (FinishForRangeVarDecl(*this, BeginVar, RangeRef.get(), ColonLoc,
+ BeginExpr = BeginRangeRef;
+ if (FinishForRangeVarDecl(*this, BeginVar, BeginRangeRef.get(),
ColonLoc,
diag::err_for_range_iter_deduction_failure)) {
NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
return StmtError();
@@ -1391,7 +1398,7 @@
}
// end-expr is __range + __bound.
- EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, RangeRef.get(),
+ EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, EndRangeRef.get(),
BoundExpr.get());
if (EndExpr.isInvalid())
return StmtError();
@@ -1431,14 +1438,14 @@
}
BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,
- BEF_begin, BeginNameInfo,
- BeginMemberLookup,
RangeRef.get());
+ BEF_begin, BeginNameInfo,
+ BeginMemberLookup,
BeginRangeRef.get());
if (BeginExpr.isInvalid())
return StmtError();
EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
BEF_end, EndNameInfo,
- EndMemberLookup, RangeRef.get());
+ EndMemberLookup,
EndRangeRef.get());
if (EndExpr.isInvalid())
return StmtError();
}
@@ -1458,12 +1465,18 @@
BuildDeclaratorGroup(BeginEndDecls, 2,
/*TypeMayContainAuto=*/false);
BeginEndDecl = ActOnDeclStmt(BeginEndGroup, ColonLoc, ColonLoc);
- ExprResult BeginRef = BuildDeclRefExpr(BeginVar,
- BeginType.getNonReferenceType(),
+ const QualType BeginRefNonRefType = BeginType.getNonReferenceType();
+ ExprResult BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
ExprResult EndRef = BuildDeclRefExpr(EndVar,
EndType.getNonReferenceType(),
VK_LValue, ColonLoc);
+ if (EndRef.isInvalid())
+ return StmtError();
+
// Build and check __begin != __end expression.
NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
BeginRef.get(), EndRef.get());
@@ -1477,6 +1490,11 @@
}
// Build and check ++__begin expression.
+ BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
IncrExpr = ActOnUnaryOp(S, ColonLoc, tok::plusplus, BeginRef.get());
IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
if (IncrExpr.isInvalid()) {
@@ -1485,6 +1503,11 @@
}
// Build and check *__begin expression.
+ BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
ExprResult DerefExpr = ActOnUnaryOp(S, ColonLoc, tok::star,
BeginRef.get());
if (DerefExpr.isInvalid()) {
NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp (revision 141126)
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp (working copy)
@@ -453,7 +453,6 @@
case Stmt::CXXBindTemporaryExprClass:
case Stmt::CXXCatchStmtClass:
case Stmt::CXXDependentScopeMemberExprClass:
- case Stmt::CXXForRangeStmtClass:
case Stmt::CXXPseudoDestructorExprClass:
case Stmt::CXXTemporaryObjectExprClass:
case Stmt::CXXThrowExprClass:
@@ -501,6 +500,7 @@
case Stmt::CaseStmtClass:
case Stmt::CompoundStmtClass:
case Stmt::ContinueStmtClass:
+ case Stmt::CXXForRangeStmtClass:
case Stmt::DefaultStmtClass:
case Stmt::DoStmtClass:
case Stmt::ForStmtClass:
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CoreEngine.cpp (revision 141126)
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp (working copy)
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/Index/TranslationUnit.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/StmtCXX.h"
#include "llvm/Support/Casting.h"
#include "llvm/ADT/DenseMap.h"
using namespace clang;
@@ -349,6 +350,10 @@
HandleBranch(cast<DoStmt>(Term)->getCond(), Term, B, Pred);
return;
+ case Stmt::CXXForRangeStmtClass:
+ HandleBranch(cast<CXXForRangeStmt>(Term)->getCond(), Term, B,
Pred);
+ return;
+
case Stmt::ForStmtClass:
HandleBranch(cast<ForStmt>(Term)->getCond(), Term, B, Pred);
return;
Index: test/Analysis/misc-ps-cxx0x.cpp
===================================================================
--- test/Analysis/misc-ps-cxx0x.cpp (revision 141126)
+++ test/Analysis/misc-ps-cxx0x.cpp (working copy)
@@ -9,3 +9,27 @@
*p = 0xDEADBEEF; // expected-warning {{null}}
}
+// Test for correct handling of C++ ForRange statement.
+void test1() {
+ int array[2] = { 1, 2 };
+ int j = 0;
+ for ( int i : array )
+ j += i;
+
+ int *p = 0;
+ *p = 0xDEADBEEF; // expected-warning {{null}}
+}
+
+void test2() {
+ int array[2] = { 1, 2 };
+ int j = 0;
+ for (int i : array)
+ j += i;
+
+ if (j == 3)
+ return;
+
+ int *p = 0;
+ *p = 0xDEADBEEF; // no-warning
+}
+
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 141174)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -1337,12 +1337,19 @@
if (!BeginEndDecl.get() && !RangeVarType->isDependentType()) {
SourceLocation RangeLoc = RangeVar->getLocation();
- ExprResult RangeRef = BuildDeclRefExpr(RangeVar,
- RangeVarType.getNonReferenceType(),
- VK_LValue, ColonLoc);
- if (RangeRef.isInvalid())
+ const QualType RangeVarNonRefType = RangeVarType.getNonReferenceType();
+
+ ExprResult BeginRangeRef = BuildDeclRefExpr(RangeVar, RangeVarNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRangeRef.isInvalid())
return StmtError();
+ ExprResult EndRangeRef = BuildDeclRefExpr(RangeVar, RangeVarNonRefType,
+ VK_LValue, ColonLoc);
+
+ if (EndRangeRef.isInvalid())
+ return StmtError();
+
QualType AutoType = Context.getAutoDeductType();
Expr *Range = RangeVar->getInit();
if (!Range)
@@ -1368,8 +1375,8 @@
// the program is ill-formed;
// begin-expr is __range.
- BeginExpr = RangeRef;
- if (FinishForRangeVarDecl(*this, BeginVar, RangeRef.get(), ColonLoc,
+ BeginExpr = BeginRangeRef;
+ if (FinishForRangeVarDecl(*this, BeginVar, BeginRangeRef.get(), ColonLoc,
diag::err_for_range_iter_deduction_failure)) {
NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
return StmtError();
@@ -1391,7 +1398,7 @@
}
// end-expr is __range + __bound.
- EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, RangeRef.get(),
+ EndExpr = ActOnBinOp(S, ColonLoc, tok::plus, EndRangeRef.get(),
BoundExpr.get());
if (EndExpr.isInvalid())
return StmtError();
@@ -1431,14 +1438,14 @@
}
BeginExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, BeginVar,
- BEF_begin, BeginNameInfo,
- BeginMemberLookup, RangeRef.get());
+ BEF_begin, BeginNameInfo,
+ BeginMemberLookup, BeginRangeRef.get());
if (BeginExpr.isInvalid())
return StmtError();
EndExpr = BuildForRangeBeginEndCall(*this, S, ColonLoc, EndVar,
BEF_end, EndNameInfo,
- EndMemberLookup, RangeRef.get());
+ EndMemberLookup, EndRangeRef.get());
if (EndExpr.isInvalid())
return StmtError();
}
@@ -1458,12 +1465,18 @@
BuildDeclaratorGroup(BeginEndDecls, 2, /*TypeMayContainAuto=*/false);
BeginEndDecl = ActOnDeclStmt(BeginEndGroup, ColonLoc, ColonLoc);
- ExprResult BeginRef = BuildDeclRefExpr(BeginVar,
- BeginType.getNonReferenceType(),
+ const QualType BeginRefNonRefType = BeginType.getNonReferenceType();
+ ExprResult BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
ExprResult EndRef = BuildDeclRefExpr(EndVar, EndType.getNonReferenceType(),
VK_LValue, ColonLoc);
+ if (EndRef.isInvalid())
+ return StmtError();
+
// Build and check __begin != __end expression.
NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
BeginRef.get(), EndRef.get());
@@ -1477,6 +1490,11 @@
}
// Build and check ++__begin expression.
+ BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
IncrExpr = ActOnUnaryOp(S, ColonLoc, tok::plusplus, BeginRef.get());
IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
if (IncrExpr.isInvalid()) {
@@ -1485,6 +1503,11 @@
}
// Build and check *__begin expression.
+ BeginRef = BuildDeclRefExpr(BeginVar, BeginRefNonRefType,
+ VK_LValue, ColonLoc);
+ if (BeginRef.isInvalid())
+ return StmtError();
+
ExprResult DerefExpr = ActOnUnaryOp(S, ColonLoc, tok::star,
BeginRef.get());
if (DerefExpr.isInvalid()) {
NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp (revision 141126)
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp (working copy)
@@ -453,7 +453,6 @@
case Stmt::CXXBindTemporaryExprClass:
case Stmt::CXXCatchStmtClass:
case Stmt::CXXDependentScopeMemberExprClass:
- case Stmt::CXXForRangeStmtClass:
case Stmt::CXXPseudoDestructorExprClass:
case Stmt::CXXTemporaryObjectExprClass:
case Stmt::CXXThrowExprClass:
@@ -501,6 +500,7 @@
case Stmt::CaseStmtClass:
case Stmt::CompoundStmtClass:
case Stmt::ContinueStmtClass:
+ case Stmt::CXXForRangeStmtClass:
case Stmt::DefaultStmtClass:
case Stmt::DoStmtClass:
case Stmt::ForStmtClass:
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CoreEngine.cpp (revision 141126)
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp (working copy)
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/Index/TranslationUnit.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/StmtCXX.h"
#include "llvm/Support/Casting.h"
#include "llvm/ADT/DenseMap.h"
using namespace clang;
@@ -349,6 +350,10 @@
HandleBranch(cast<DoStmt>(Term)->getCond(), Term, B, Pred);
return;
+ case Stmt::CXXForRangeStmtClass:
+ HandleBranch(cast<CXXForRangeStmt>(Term)->getCond(), Term, B, Pred);
+ return;
+
case Stmt::ForStmtClass:
HandleBranch(cast<ForStmt>(Term)->getCond(), Term, B, Pred);
return;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
