[cfe-commits] [PATCH][Review Request] Handle CXXForRangeStmt

Jim Goodnow II Jim at TheGoodnows.net
Thu Oct 6 03:19:55 PDT 2011


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;

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ForRange.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111006/f2e3a90e/attachment.ksh>


More information about the cfe-commits mailing list