r273309 - Refactor scope building in JumpDiagnostics for simplicity. This fixes a

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 21 13:10:12 PDT 2016


Author: rsmith
Date: Tue Jun 21 15:10:11 2016
New Revision: 273309

URL: http://llvm.org/viewvc/llvm-project?rev=273309&view=rev
Log:
Refactor scope building in JumpDiagnostics for simplicity. This fixes a
(currently theoretical) bug where recursive calls to BuildScopeInformation
would do the wrong thing if the type of the statement is one of the kinds with
special handling, but this is not currently observable because the relevant
recursive calls happen to all be for CompoundStmts. (This becomes visible with
the C++17 'constexpr if' feature, where we get a protected scope for the 'then'
/ 'else' cases of some 'if's, and don't necessarily have a corresponding
compound statement.)

Modified:
    cfe/trunk/lib/Sema/JumpDiagnostics.cpp

Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=273309&r1=273308&r2=273309&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original)
+++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Tue Jun 21 15:10:11 2016
@@ -270,7 +270,8 @@ void JumpScopeChecker::BuildScopeInforma
 /// coherent VLA scope with a specified parent node.  Walk through the
 /// statements, adding any labels or gotos to LabelAndGotoScopes and recursively
 /// walking the AST as needed.
-void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned &origParentScope) {
+void JumpScopeChecker::BuildScopeInformation(Stmt *S,
+                                             unsigned &origParentScope) {
   // If this is a statement, rather than an expression, scopes within it don't
   // propagate out into the enclosing scope.  Otherwise we have to worry
   // about block literals, which have the lifetime of their enclosing statement.
@@ -320,57 +321,186 @@ void JumpScopeChecker::BuildScopeInforma
 
   case Stmt::CXXTryStmtClass: {
     CXXTryStmt *TS = cast<CXXTryStmt>(S);
-    unsigned newParentScope;
-    Scopes.push_back(GotoScope(ParentScope,
-                               diag::note_protected_by_cxx_try,
-                               diag::note_exits_cxx_try,
-                               TS->getSourceRange().getBegin()));
-    if (Stmt *TryBlock = TS->getTryBlock())
-      BuildScopeInformation(TryBlock, (newParentScope = Scopes.size()-1));
+    {
+      unsigned NewParentScope = Scopes.size();
+      Scopes.push_back(GotoScope(ParentScope,
+                                 diag::note_protected_by_cxx_try,
+                                 diag::note_exits_cxx_try,
+                                 TS->getSourceRange().getBegin()));
+      if (Stmt *TryBlock = TS->getTryBlock())
+        BuildScopeInformation(TryBlock, NewParentScope);
+    }
 
     // Jump from the catch into the try is not allowed either.
     for (unsigned I = 0, E = TS->getNumHandlers(); I != E; ++I) {
       CXXCatchStmt *CS = TS->getHandler(I);
+      unsigned NewParentScope = Scopes.size();
       Scopes.push_back(GotoScope(ParentScope,
                                  diag::note_protected_by_cxx_catch,
                                  diag::note_exits_cxx_catch,
                                  CS->getSourceRange().getBegin()));
-      BuildScopeInformation(CS->getHandlerBlock(),
-                            (newParentScope = Scopes.size()-1));
+      BuildScopeInformation(CS->getHandlerBlock(), NewParentScope);
     }
     return;
   }
 
   case Stmt::SEHTryStmtClass: {
     SEHTryStmt *TS = cast<SEHTryStmt>(S);
-    unsigned newParentScope;
-    Scopes.push_back(GotoScope(ParentScope,
-                               diag::note_protected_by_seh_try,
-                               diag::note_exits_seh_try,
-                               TS->getSourceRange().getBegin()));
-    if (Stmt *TryBlock = TS->getTryBlock())
-      BuildScopeInformation(TryBlock, (newParentScope = Scopes.size()-1));
+    {
+      unsigned NewParentScope = Scopes.size();
+      Scopes.push_back(GotoScope(ParentScope,
+                                 diag::note_protected_by_seh_try,
+                                 diag::note_exits_seh_try,
+                                 TS->getSourceRange().getBegin()));
+      if (Stmt *TryBlock = TS->getTryBlock())
+        BuildScopeInformation(TryBlock, NewParentScope);
+    }
 
     // Jump from __except or __finally into the __try are not allowed either.
     if (SEHExceptStmt *Except = TS->getExceptHandler()) {
+      unsigned NewParentScope = Scopes.size();
       Scopes.push_back(GotoScope(ParentScope,
                                  diag::note_protected_by_seh_except,
                                  diag::note_exits_seh_except,
                                  Except->getSourceRange().getBegin()));
-      BuildScopeInformation(Except->getBlock(),
-                            (newParentScope = Scopes.size()-1));
+      BuildScopeInformation(Except->getBlock(), NewParentScope);
     } else if (SEHFinallyStmt *Finally = TS->getFinallyHandler()) {
+      unsigned NewParentScope = Scopes.size();
       Scopes.push_back(GotoScope(ParentScope,
                                  diag::note_protected_by_seh_finally,
                                  diag::note_exits_seh_finally,
                                  Finally->getSourceRange().getBegin()));
-      BuildScopeInformation(Finally->getBlock(),
-                            (newParentScope = Scopes.size()-1));
+      BuildScopeInformation(Finally->getBlock(), NewParentScope);
     }
 
     return;
   }
 
+  case Stmt::DeclStmtClass: {
+    // If this is a declstmt with a VLA definition, it defines a scope from here
+    // to the end of the containing context.
+    DeclStmt *DS = cast<DeclStmt>(S);
+    // The decl statement creates a scope if any of the decls in it are VLAs
+    // or have the cleanup attribute.
+    for (auto *I : DS->decls())
+      BuildScopeInformation(I, origParentScope);
+    return;
+  }
+
+  case Stmt::ObjCAtTryStmtClass: {
+    // Disallow jumps into any part of an @try statement by pushing a scope and
+    // walking all sub-stmts in that scope.
+    ObjCAtTryStmt *AT = cast<ObjCAtTryStmt>(S);
+    // Recursively walk the AST for the @try part.
+    {
+      unsigned NewParentScope = Scopes.size();
+      Scopes.push_back(GotoScope(ParentScope,
+                                 diag::note_protected_by_objc_try,
+                                 diag::note_exits_objc_try,
+                                 AT->getAtTryLoc()));
+      if (Stmt *TryPart = AT->getTryBody())
+        BuildScopeInformation(TryPart, NewParentScope);
+    }
+
+    // Jump from the catch to the finally or try is not valid.
+    for (unsigned I = 0, N = AT->getNumCatchStmts(); I != N; ++I) {
+      ObjCAtCatchStmt *AC = AT->getCatchStmt(I);
+      unsigned NewParentScope = Scopes.size();
+      Scopes.push_back(GotoScope(ParentScope,
+                                 diag::note_protected_by_objc_catch,
+                                 diag::note_exits_objc_catch,
+                                 AC->getAtCatchLoc()));
+      // @catches are nested and it isn't
+      BuildScopeInformation(AC->getCatchBody(), NewParentScope);
+    }
+
+    // Jump from the finally to the try or catch is not valid.
+    if (ObjCAtFinallyStmt *AF = AT->getFinallyStmt()) {
+      unsigned NewParentScope = Scopes.size();
+      Scopes.push_back(GotoScope(ParentScope,
+                                 diag::note_protected_by_objc_finally,
+                                 diag::note_exits_objc_finally,
+                                 AF->getAtFinallyLoc()));
+      BuildScopeInformation(AF, NewParentScope);
+    }
+
+    return;
+  }
+
+  case Stmt::ObjCAtSynchronizedStmtClass: {
+    // Disallow jumps into the protected statement of an @synchronized, but
+    // allow jumps into the object expression it protects.
+    ObjCAtSynchronizedStmt *AS = cast<ObjCAtSynchronizedStmt>(S);
+    // Recursively walk the AST for the @synchronized object expr, it is
+    // evaluated in the normal scope.
+    BuildScopeInformation(AS->getSynchExpr(), ParentScope);
+
+    // Recursively walk the AST for the @synchronized part, protected by a new
+    // scope.
+    unsigned NewParentScope = Scopes.size();
+    Scopes.push_back(GotoScope(ParentScope,
+                               diag::note_protected_by_objc_synchronized,
+                               diag::note_exits_objc_synchronized,
+                               AS->getAtSynchronizedLoc()));
+    BuildScopeInformation(AS->getSynchBody(), NewParentScope);
+    return;
+  }
+
+  case Stmt::ObjCAutoreleasePoolStmtClass: {
+    // Disallow jumps into the protected statement of an @autoreleasepool.
+    ObjCAutoreleasePoolStmt *AS = cast<ObjCAutoreleasePoolStmt>(S);
+    // Recursively walk the AST for the @autoreleasepool part, protected by a
+    // new scope.
+    unsigned NewParentScope = Scopes.size();
+    Scopes.push_back(GotoScope(ParentScope,
+                               diag::note_protected_by_objc_autoreleasepool,
+                               diag::note_exits_objc_autoreleasepool,
+                               AS->getAtLoc()));
+    BuildScopeInformation(AS->getSubStmt(), NewParentScope);
+    return;
+  }
+
+  case Stmt::ExprWithCleanupsClass: {
+    // Disallow jumps past full-expressions that use blocks with
+    // non-trivial cleanups of their captures.  This is theoretically
+    // implementable but a lot of work which we haven't felt up to doing.
+    ExprWithCleanups *EWC = cast<ExprWithCleanups>(S);
+    for (unsigned i = 0, e = EWC->getNumObjects(); i != e; ++i) {
+      const BlockDecl *BDecl = EWC->getObject(i);
+      for (const auto &CI : BDecl->captures()) {
+        VarDecl *variable = CI.getVariable();
+        BuildScopeInformation(variable, BDecl, origParentScope);
+      }
+    }
+    break;
+  }
+
+  case Stmt::MaterializeTemporaryExprClass: {
+    // Disallow jumps out of scopes containing temporaries lifetime-extended to
+    // automatic storage duration.
+    MaterializeTemporaryExpr *MTE = cast<MaterializeTemporaryExpr>(S);
+    if (MTE->getStorageDuration() == SD_Automatic) {
+      SmallVector<const Expr *, 4> CommaLHS;
+      SmallVector<SubobjectAdjustment, 4> Adjustments;
+      const Expr *ExtendedObject =
+          MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(
+              CommaLHS, Adjustments);
+      if (ExtendedObject->getType().isDestructedType()) {
+        Scopes.push_back(GotoScope(ParentScope, 0,
+                                   diag::note_exits_temporary_dtor,
+                                   ExtendedObject->getExprLoc()));
+        origParentScope = Scopes.size()-1;
+      }
+    }
+    break;
+  }
+
+  case Stmt::CaseStmtClass:
+  case Stmt::DefaultStmtClass:
+  case Stmt::LabelStmtClass:
+    LabelAndGotoScopes[S] = ParentScope;
+    break;
+
   default:
     break;
   }
@@ -401,117 +531,6 @@ void JumpScopeChecker::BuildScopeInforma
       SubStmt = Next;
     }
 
-    // If this is a declstmt with a VLA definition, it defines a scope from here
-    // to the end of the containing context.
-    if (DeclStmt *DS = dyn_cast<DeclStmt>(SubStmt)) {
-      // The decl statement creates a scope if any of the decls in it are VLAs
-      // or have the cleanup attribute.
-      for (auto *I : DS->decls())
-        BuildScopeInformation(I, ParentScope);
-      continue;
-    }
-    // Disallow jumps into any part of an @try statement by pushing a scope and
-    // walking all sub-stmts in that scope.
-    if (ObjCAtTryStmt *AT = dyn_cast<ObjCAtTryStmt>(SubStmt)) {
-      unsigned newParentScope;
-      // Recursively walk the AST for the @try part.
-      Scopes.push_back(GotoScope(ParentScope,
-                                 diag::note_protected_by_objc_try,
-                                 diag::note_exits_objc_try,
-                                 AT->getAtTryLoc()));
-      if (Stmt *TryPart = AT->getTryBody())
-        BuildScopeInformation(TryPart, (newParentScope = Scopes.size()-1));
-
-      // Jump from the catch to the finally or try is not valid.
-      for (unsigned I = 0, N = AT->getNumCatchStmts(); I != N; ++I) {
-        ObjCAtCatchStmt *AC = AT->getCatchStmt(I);
-        Scopes.push_back(GotoScope(ParentScope,
-                                   diag::note_protected_by_objc_catch,
-                                   diag::note_exits_objc_catch,
-                                   AC->getAtCatchLoc()));
-        // @catches are nested and it isn't
-        BuildScopeInformation(AC->getCatchBody(),
-                              (newParentScope = Scopes.size()-1));
-      }
-
-      // Jump from the finally to the try or catch is not valid.
-      if (ObjCAtFinallyStmt *AF = AT->getFinallyStmt()) {
-        Scopes.push_back(GotoScope(ParentScope,
-                                   diag::note_protected_by_objc_finally,
-                                   diag::note_exits_objc_finally,
-                                   AF->getAtFinallyLoc()));
-        BuildScopeInformation(AF, (newParentScope = Scopes.size()-1));
-      }
-
-      continue;
-    }
-
-    unsigned newParentScope;
-    // Disallow jumps into the protected statement of an @synchronized, but
-    // allow jumps into the object expression it protects.
-    if (ObjCAtSynchronizedStmt *AS =
-            dyn_cast<ObjCAtSynchronizedStmt>(SubStmt)) {
-      // Recursively walk the AST for the @synchronized object expr, it is
-      // evaluated in the normal scope.
-      BuildScopeInformation(AS->getSynchExpr(), ParentScope);
-
-      // Recursively walk the AST for the @synchronized part, protected by a new
-      // scope.
-      Scopes.push_back(GotoScope(ParentScope,
-                                 diag::note_protected_by_objc_synchronized,
-                                 diag::note_exits_objc_synchronized,
-                                 AS->getAtSynchronizedLoc()));
-      BuildScopeInformation(AS->getSynchBody(),
-                            (newParentScope = Scopes.size()-1));
-      continue;
-    }
-
-    // Disallow jumps into the protected statement of an @autoreleasepool.
-    if (ObjCAutoreleasePoolStmt *AS =
-            dyn_cast<ObjCAutoreleasePoolStmt>(SubStmt)) {
-      // Recursively walk the AST for the @autoreleasepool part, protected by a
-      // new scope.
-      Scopes.push_back(GotoScope(ParentScope,
-                                 diag::note_protected_by_objc_autoreleasepool,
-                                 diag::note_exits_objc_autoreleasepool,
-                                 AS->getAtLoc()));
-      BuildScopeInformation(AS->getSubStmt(),
-                            (newParentScope = Scopes.size() - 1));
-      continue;
-    }
-
-    // Disallow jumps past full-expressions that use blocks with
-    // non-trivial cleanups of their captures.  This is theoretically
-    // implementable but a lot of work which we haven't felt up to doing.
-    if (ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(SubStmt)) {
-      for (unsigned i = 0, e = EWC->getNumObjects(); i != e; ++i) {
-        const BlockDecl *BDecl = EWC->getObject(i);
-        for (const auto &CI : BDecl->captures()) {
-          VarDecl *variable = CI.getVariable();
-          BuildScopeInformation(variable, BDecl, ParentScope);
-        }
-      }
-    }
-
-    // Disallow jumps out of scopes containing temporaries lifetime-extended to
-    // automatic storage duration.
-    if (MaterializeTemporaryExpr *MTE =
-            dyn_cast<MaterializeTemporaryExpr>(SubStmt)) {
-      if (MTE->getStorageDuration() == SD_Automatic) {
-        SmallVector<const Expr *, 4> CommaLHS;
-        SmallVector<SubobjectAdjustment, 4> Adjustments;
-        const Expr *ExtendedObject =
-            MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(
-                CommaLHS, Adjustments);
-        if (ExtendedObject->getType().isDestructedType()) {
-          Scopes.push_back(GotoScope(ParentScope, 0,
-                                     diag::note_exits_temporary_dtor,
-                                     ExtendedObject->getExprLoc()));
-          ParentScope = Scopes.size()-1;
-        }
-      }
-    }
-
     // Recursively walk the AST.
     BuildScopeInformation(SubStmt, ParentScope);
   }




More information about the cfe-commits mailing list