Keep an explicit stack of function and block scopes, each element of

which has the label map, switch statement stack, etc. Previously, we
had a single set of maps in Sema (for the function) along with a stack
of block scopes. However, this lead to funky behavior with nested
functions, e.g., in the member functions of local classes.

The explicit-stack approach is far cleaner, and we retain a 1-element
cache so that we're not malloc/free'ing every time we enter a
function. Fixes PR6382.

Also, tweaked the unused-variable warning suppression logic to look at
errors within a given Scope rather than within a given function. The
prior code wasn't looking at the right number-of-errors count when
dealing with blocks, since the block's count would be deallocated
before we got to ActOnPopScope. This approach works with nested
blocks/functions, and gives tighter error recovery.

llvm-svn: 97518

clang/include/clang/Parse/Scope.h

@@ -129,6 +129,9 @@ private:
   typedef llvm::SmallVector<Action::DeclPtrTy, 2> UsingDirectivesTy;
   UsingDirectivesTy UsingDirectives;
 
+  /// \brief The number of errors at the start of the given scope.
+  unsigned NumErrorsAtStart;
+  
 public:
   Scope(Scope *Parent, unsigned ScopeFlags) {
     Init(Parent, ScopeFlags);
@@ -208,6 +211,10 @@ public:
   void* getEntity() const { return Entity; }
   void setEntity(void *E) { Entity = E; }
 
+  /// \brief Retrieve the number of errors that had been emitted when we
+  /// entered this scope.
+  unsigned getNumErrorsAtStart() const { return NumErrorsAtStart; }
+  
   /// isClassScope - Return true if this scope is a class/struct/union scope.
   bool isClassScope() const {
     return (getFlags() & Scope::ClassScope);
@@ -300,6 +307,7 @@ public:
     DeclsInScope.clear();
     UsingDirectives.clear();
     Entity = 0;
+    NumErrorsAtStart = 0;
   }
 };
 

clang/lib/Parse/Parser.cpp

@@ -274,6 +274,7 @@ void Parser::EnterScope(unsigned ScopeFlags) {
   } else {
     CurScope = new Scope(CurScope, ScopeFlags);
   }
+  CurScope->NumErrorsAtStart = Diags.getNumErrors();
 }
 
 /// ExitScope - Pop a scope off the scope stack.

clang/lib/Sema/Sema.cpp

@@ -27,6 +27,17 @@
 #include "clang/Basic/TargetInfo.h"
 using namespace clang;
 
+FunctionScopeInfo::~FunctionScopeInfo() { }
+
+void FunctionScopeInfo::Clear(unsigned NumErrors) {
+  NeedsScopeChecking = false;
+  LabelMap.clear();
+  SwitchStack.clear();
+  NumErrorsAtStartOfFunction = NumErrors;
+}
+
+BlockScopeInfo::~BlockScopeInfo() { }
+
 static inline RecordDecl *CreateStructDecl(ASTContext &C, const char *Name) {
   if (C.getLangOptions().CPlusPlus)
     return CXXRecordDecl::Create(C, TagDecl::TK_struct,
@@ -116,7 +127,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     LangOpts(pp.getLangOptions()), PP(pp), Context(ctxt), Consumer(consumer),
     Diags(PP.getDiagnostics()), SourceMgr(PP.getSourceManager()),
     ExternalSource(0), CodeCompleter(CodeCompleter), CurContext(0), 
-    CurBlock(0), PackContext(0), ParsingDeclDepth(0),
+    PackContext(0), TopFunctionScope(0), ParsingDeclDepth(0),
     IdResolver(pp.getLangOptions()), StdNamespace(0), StdBadAlloc(0),
     GlobalNewDeleteDeclared(false), 
     CompleteTranslationUnit(CompleteTranslationUnit),
@@ -127,8 +138,6 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
   if (getLangOptions().CPlusPlus)
     FieldCollector.reset(new CXXFieldCollector());
 
-  NumErrorsAtStartOfFunction = 0;
-  
   // Tell diagnostics how to render things from the AST library.
   PP.getDiagnostics().SetArgToStringFn(&FormatASTNodeDiagnosticArgument, 
                                        &Context);
@@ -140,6 +149,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
 Sema::~Sema() {
   if (PackContext) FreePackedContext();
   delete TheTargetAttributesSema;
+  while (!FunctionScopes.empty())
+    PopFunctionOrBlockScope();
 }
 
 /// ImpCastExprToType - If Expr is not of type 'Type', insert an implicit cast.
@@ -344,6 +355,51 @@ Sema::Diag(SourceLocation Loc, const PartialDiagnostic& PD) {
   return Builder;
 }
 
+
+/// \brief Enter a new function scope
+void Sema::PushFunctionScope() {
+  if (FunctionScopes.empty()) {
+    // Use the "top" function scope rather than having to allocate memory for
+    // a new scope.
+    TopFunctionScope.Clear(getDiagnostics().getNumErrors());
+    FunctionScopes.push_back(&TopFunctionScope);
+    return;
+  }
+  
+  FunctionScopes.push_back(
+                      new FunctionScopeInfo(getDiagnostics().getNumErrors()));
+}
+
+void Sema::PushBlockScope(Scope *BlockScope, BlockDecl *Block) {
+  FunctionScopes.push_back(new BlockScopeInfo(getDiagnostics().getNumErrors(),
+                                              BlockScope, Block));
+}
+
+void Sema::PopFunctionOrBlockScope() {
+  if (FunctionScopes.back() != &TopFunctionScope)
+    delete FunctionScopes.back();
+  else
+    TopFunctionScope.Clear(getDiagnostics().getNumErrors());
+  
+  FunctionScopes.pop_back();
+}
+
+/// \brief Determine whether any errors occurred within this function/method/
+/// block.
+bool Sema::hasAnyErrorsInThisFunction() const {
+  unsigned NumErrors = TopFunctionScope.NumErrorsAtStartOfFunction;
+  if (!FunctionScopes.empty())
+    NumErrors = FunctionScopes.back()->NumErrorsAtStartOfFunction;
+  return NumErrors != getDiagnostics().getNumErrors();
+}
+
+BlockScopeInfo *Sema::getCurBlock() {
+  if (FunctionScopes.empty())
+    return 0;
+  
+  return dyn_cast<BlockScopeInfo>(FunctionScopes.back());  
+}
+
 void Sema::ActOnComment(SourceRange Comment) {
   Context.Comments.push_back(Comment);
 }

clang/lib/Sema/Sema.h

@@ -111,6 +111,19 @@ namespace clang {
 /// \brief Retains information about a function, method, or block that is 
 /// currently being parsed.
 struct FunctionScopeInfo {
+  /// \brief Whether this scope information structure defined information for
+  /// a block.
+  bool IsBlockInfo;
+  
+  /// \brief Set true when a function, method contains a VLA or ObjC try block, 
+  /// which introduce scopes that need to be checked for goto conditions.  If a
+  /// function does not contain this, then it need not have the jump checker run on it.
+  bool NeedsScopeChecking;
+    
+  /// \brief The number of errors that had occurred before starting this
+  /// function or block.
+  unsigned NumErrorsAtStartOfFunction;
+  
   /// LabelMap - This is a mapping from label identifiers to the LabelStmt for
   /// it (which acts like the label decl in some ways).  Forward referenced
   /// labels have a LabelStmt created for them with a null location & SubStmt.
@@ -120,11 +133,15 @@ struct FunctionScopeInfo {
   /// block.
   llvm::SmallVector<SwitchStmt*, 8> SwitchStack;  
   
-  /// \brief Whether this scope information structure defined information for
+  FunctionScopeInfo(unsigned NumErrors) 
-  /// a block.
+    : IsBlockInfo(false), NeedsScopeChecking(false), 
-  bool IsBlockInfo;
+      NumErrorsAtStartOfFunction(NumErrors) { }
 
-  FunctionScopeInfo() : IsBlockInfo(false) { }
+  virtual ~FunctionScopeInfo();
+
+  /// \brief Clear out the information in this function scope, making it
+  /// suitable for reuse.
+  void Clear(unsigned NumErrors);
   
   static bool classof(const FunctionScopeInfo *FSI) { return true; }  
 };
@@ -147,17 +164,14 @@ struct BlockScopeInfo : FunctionScopeInfo {
   /// return types, if any, in the block body.
   QualType ReturnType;
 
-  /// SavedNumErrorsAtStartOfFunction - This is the value of the
+  BlockScopeInfo(unsigned NumErrors, Scope *BlockScope, BlockDecl *Block) 
-  /// NumErrorsAtStartOfFunction variable at the point when the block started.
+    : FunctionScopeInfo(NumErrors), hasPrototype(false), isVariadic(false), 
-  unsigned SavedNumErrorsAtStartOfFunction;
+      hasBlockDeclRefExprs(false), TheDecl(Block), TheScope(BlockScope) 
+  {
+    IsBlockInfo = true; 
+  }
 
-  /// SavedFunctionNeedsScopeChecking - This is the value of
+  virtual ~BlockScopeInfo();
-  /// CurFunctionNeedsScopeChecking at the point when the block started.
-  bool SavedFunctionNeedsScopeChecking;
-
-  BlockScopeInfo *PrevBlockInfo;
-  
-  BlockScopeInfo() : FunctionScopeInfo() { IsBlockInfo = true; }
 
   static bool classof(const FunctionScopeInfo *FSI) { return FSI->IsBlockInfo; }
   static bool classof(const BlockScopeInfo *BSI) { return true; }
@@ -219,45 +233,25 @@ public:
   /// CurContext - This is the current declaration context of parsing.
   DeclContext *CurContext;
 
-  /// CurBlock - If inside of a block definition, this contains a pointer to
-  /// the active block object that represents it.
-  BlockScopeInfo *CurBlock;
-
   /// PackContext - Manages the stack for #pragma pack. An alignment
   /// of 0 indicates default alignment.
   void *PackContext; // Really a "PragmaPackStack*"
 
-  /// FunctionLabelMap - This is a mapping from label identifiers to the
+  /// \brief Stack containing information about each of the nested function,
-  /// LabelStmt for it (which acts like the label decl in some ways).  Forward
+  /// block, and method scopes that are currently active.
-  /// referenced labels have a LabelStmt created for them with a null location &
+  llvm::SmallVector<FunctionScopeInfo *, 4> FunctionScopes;
-  /// SubStmt.
-  ///
-  /// Note that this should always be accessed through getLabelMap() in order
-  /// to handle blocks properly.
-  llvm::DenseMap<IdentifierInfo*, LabelStmt*> FunctionLabelMap;
   
-  /// FunctionSwitchStack - This is the current set of active switch statements
+  /// \brief Cached function scope object used for the top function scope
-  /// in the top level function.  Clients should always use getSwitchStack() to
+  /// and when there is no function scope (in error cases).
-  /// handle the case when they are in a block.
+  ///
-  llvm::SmallVector<SwitchStmt*, 8> FunctionSwitchStack;
+  /// This should never be accessed directly; rather, it's address will be 
+  /// pushed into \c FunctionScopes when we want to re-use it.
+  FunctionScopeInfo TopFunctionScope;
   
   /// ExprTemporaries - This is the stack of temporaries that are created by
   /// the current full expression.
   llvm::SmallVector<CXXTemporary*, 8> ExprTemporaries;
 
-  /// NumErrorsAtStartOfFunction - This is the number of errors that were
-  /// emitted to the diagnostics object at the start of the current
-  /// function/method definition.  If no additional errors are emitted by the
-  /// end of the function, we assume the AST is well formed enough to be
-  /// worthwhile to emit control flow diagnostics. 
-  unsigned NumErrorsAtStartOfFunction;
-  
-  /// CurFunctionNeedsScopeChecking - This is set to true when a function or
-  /// ObjC method body contains a VLA or an ObjC try block, which introduce
-  /// scopes that need to be checked for goto conditions.  If a function does
-  /// not contain this, then it need not have the jump checker run on it.
-  bool CurFunctionNeedsScopeChecking;
-
   /// ExtVectorDecls - This is a list all the extended vector types. This allows
   /// us to associate a raw vector type with one of the ext_vector type names.
   /// This is only necessary for issuing pretty diagnostics.
@@ -633,18 +627,42 @@ public:
 
   virtual void ActOnEndOfTranslationUnit();
 
+  void PushFunctionScope();
+  void PushBlockScope(Scope *BlockScope, BlockDecl *Block);
+  void PopFunctionOrBlockScope();
+  
   /// getLabelMap() - Return the current label map.  If we're in a block, we
   /// return it.
   llvm::DenseMap<IdentifierInfo*, LabelStmt*> &getLabelMap() {
-    return CurBlock ? CurBlock->LabelMap : FunctionLabelMap;
+    if (FunctionScopes.empty())
+      return TopFunctionScope.LabelMap;
+    
+    return FunctionScopes.back()->LabelMap;
   }
 
   /// getSwitchStack - This is returns the switch stack for the current block or
   /// function.
   llvm::SmallVector<SwitchStmt*,8> &getSwitchStack() {
-    return CurBlock ? CurBlock->SwitchStack : FunctionSwitchStack;
+    if (FunctionScopes.empty())
+      return TopFunctionScope.SwitchStack;
+    
+    return FunctionScopes.back()->SwitchStack;
   }
 
+  /// \brief Determine whether the current function or block needs scope 
+  /// checking.
+  bool &FunctionNeedsScopeChecking() {
+    if (FunctionScopes.empty())
+      return TopFunctionScope.NeedsScopeChecking;
+    
+    return FunctionScopes.back()->NeedsScopeChecking;
+  }
+  
+  bool hasAnyErrorsInThisFunction() const;
+  
+  /// \brief Retrieve the current block, if any.
+  BlockScopeInfo *getCurBlock();
+  
   /// WeakTopLevelDeclDecls - access to #pragma weak-generated Decls
   llvm::SmallVector<Decl*,2> &WeakTopLevelDecls() { return WeakTopLevelDecl; }
 

clang/lib/Sema/SemaChecking.cpp

@@ -506,6 +506,7 @@ bool Sema::SemaBuiltinVAStart(CallExpr *TheCall) {
   }
 
   // Determine whether the current function is variadic or not.
+  BlockScopeInfo *CurBlock = getCurBlock();
   bool isVariadic;
   if (CurBlock)
     isVariadic = CurBlock->isVariadic;

clang/lib/Sema/SemaCodeComplete.cpp

@@ -1137,8 +1137,9 @@ static void AddOrdinaryNameResults(Action::CodeCompletionContext CCC,
     else if (ObjCMethodDecl *Method
                                  = dyn_cast<ObjCMethodDecl>(SemaRef.CurContext))
       isVoid = Method->getResultType()->isVoidType();
-    else if (SemaRef.CurBlock && !SemaRef.CurBlock->ReturnType.isNull())
+    else if (SemaRef.getCurBlock() && 
-      isVoid = SemaRef.CurBlock->ReturnType->isVoidType();
+             !SemaRef.getCurBlock()->ReturnType.isNull())
+      isVoid = SemaRef.getCurBlock()->ReturnType->isVoidType();
     Pattern = new CodeCompletionString;
     Pattern->AddTypedTextChunk("return");
     if (!isVoid) {

clang/lib/Sema/SemaDecl.cpp

@@ -554,7 +554,7 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
 
     // Diagnose unused variables in this scope.
     if (ShouldDiagnoseUnusedDecl(D) && 
-        NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors())
+        S->getNumErrorsAtStart() == getDiagnostics().getNumErrors())
       Diag(D->getLocation(), diag::warn_unused_variable) << D->getDeclName();
     
     // Remove this name from our lexical scope.
@@ -2180,7 +2180,7 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // then it shall have block scope.
   QualType T = NewTD->getUnderlyingType();
   if (T->isVariablyModifiedType()) {
-    CurFunctionNeedsScopeChecking = true;
+    FunctionNeedsScopeChecking() = true;
 
     if (S->getFnParent() == 0) {
       bool SizeIsNegative;
@@ -2507,7 +2507,7 @@ void Sema::CheckVariableDeclaration(VarDecl *NewVD,
       // which may impact compile time.  See if we can find a better solution
       // to this, perhaps only checking functions that contain gotos in C++?
       (LangOpts.CPlusPlus && NewVD->hasLocalStorage()))
-    CurFunctionNeedsScopeChecking = true;
+    FunctionNeedsScopeChecking() = true;
 
   if ((isVM && NewVD->hasLinkage()) ||
       (T->isVariableArrayType() && NewVD->hasGlobalStorage())) {
@@ -4084,8 +4084,8 @@ Sema::DeclPtrTy Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, DeclPtrTy D) {
   else
     FD = cast<FunctionDecl>(D.getAs<Decl>());
 
-  CurFunctionNeedsScopeChecking = false;
+  // Enter a new function scope
-  NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors();
+  PushFunctionScope();
 
   // See if this is a redefinition.
   // But don't complain if we're in GNU89 mode and the previous definition
@@ -4217,11 +4217,9 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg,
 
   // Verify and clean out per-function state.
 
-  assert(&getLabelMap() == &FunctionLabelMap && "Didn't pop block right?");
-
   // Check goto/label use.
   for (llvm::DenseMap<IdentifierInfo*, LabelStmt*>::iterator
-       I = FunctionLabelMap.begin(), E = FunctionLabelMap.end(); I != E; ++I) {
+       I = getLabelMap().begin(), E = getLabelMap().end(); I != E; ++I) {
     LabelStmt *L = I->second;
 
     // Verify that we have no forward references left.  If so, there was a goto
@@ -4257,33 +4255,41 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg,
     Elements.push_back(L);
     Compound->setStmts(Context, &Elements[0], Elements.size());
   }
-  FunctionLabelMap.clear();
-
-  if (!Body) return D;
 
+  if (Body) {
     CheckUnreachable(AC);
 
-  // Verify that that gotos and switch cases don't jump into scopes illegally.
-  if (CurFunctionNeedsScopeChecking &&
-      NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors())
-    DiagnoseInvalidJumps(Body);
-
     // C++ constructors that have function-try-blocks can't have return
     // statements in the handlers of that block. (C++ [except.handle]p14)
     // Verify this.
     if (FD && isa<CXXConstructorDecl>(FD) && isa<CXXTryStmt>(Body))
       DiagnoseReturnInConstructorExceptionHandler(cast<CXXTryStmt>(Body));
     
+  // Verify that that gotos and switch cases don't jump into scopes illegally.
+  // Verify that that gotos and switch cases don't jump into scopes illegally.
+    if (FunctionNeedsScopeChecking() && !hasAnyErrorsInThisFunction())
+      DiagnoseInvalidJumps(Body);
+
     if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(dcl))
       MarkBaseAndMemberDestructorsReferenced(Destructor);
     
+    // If any errors have occurred, clear out any temporaries that may have
+    // been leftover. This ensures that these temporaries won't be picked up for
+    // deletion in some later function.
+    if (PP.getDiagnostics().hasErrorOccurred())
+      ExprTemporaries.clear();
+    
+    assert(ExprTemporaries.empty() && "Leftover temporaries in function");
+  }
+  
+  PopFunctionOrBlockScope();
+  
   // If any errors have occurred, clear out any temporaries that may have
   // been leftover. This ensures that these temporaries won't be picked up for
   // deletion in some later function.
   if (getDiagnostics().hasErrorOccurred())
     ExprTemporaries.clear();
   
-  assert(ExprTemporaries.empty() && "Leftover temporaries in function");
   return D;
 }
 

clang/lib/Sema/SemaDeclObjC.cpp

@@ -49,9 +49,6 @@ void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, DeclPtrTy D) {
   if (!MDecl)
     return;
 
-  CurFunctionNeedsScopeChecking = false;
-  NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors();
-
   // Allow the rest of sema to find private method decl implementations.
   if (MDecl->isInstanceMethod())
     AddInstanceMethodToGlobalPool(MDecl);
@@ -60,6 +57,7 @@ void Sema::ActOnStartOfObjCMethodDef(Scope *FnBodyScope, DeclPtrTy D) {
 
   // Allow all of Sema to see that we are entering a method definition.
   PushDeclContext(FnBodyScope, MDecl);
+  PushFunctionScope();
   
   // Create Decl objects for each parameter, entrring them in the scope for
   // binding to their use.
@@ -1989,7 +1987,7 @@ Sema::DeclPtrTy Sema::ActOnMethodDeclaration(
   // Make sure we can establish a context for the method.
   if (!ClassDecl) {
     Diag(MethodLoc, diag::error_missing_method_context);
-    FunctionLabelMap.clear();
+    getLabelMap().clear();
     return DeclPtrTy();
   }
   QualType resultDeclType;

clang/lib/Sema/SemaExpr.cpp

@@ -396,7 +396,7 @@ Sema::ActOnStringLiteral(const Token *StringToks, unsigned NumStringToks) {
 /// This also keeps the 'hasBlockDeclRefExprs' in the BlockScopeInfo records
 /// up-to-date.
 ///
-static bool ShouldSnapshotBlockValueReference(BlockScopeInfo *CurBlock,
+static bool ShouldSnapshotBlockValueReference(Sema &S, BlockScopeInfo *CurBlock,
                                               ValueDecl *VD) {
   // If the value is defined inside the block, we couldn't snapshot it even if
   // we wanted to.
@@ -421,8 +421,12 @@ static bool ShouldSnapshotBlockValueReference(BlockScopeInfo *CurBlock,
   // which case that outer block doesn't get "hasBlockDeclRefExprs") or it may
   // be defined outside all of the current blocks (in which case the blocks do
   // all get the bit).  Walk the nesting chain.
-  for (BlockScopeInfo *NextBlock = CurBlock->PrevBlockInfo; NextBlock;
+  for (unsigned I = S.FunctionScopes.size() - 1; I; --I) {
-       NextBlock = NextBlock->PrevBlockInfo) {
+    BlockScopeInfo *NextBlock = dyn_cast<BlockScopeInfo>(S.FunctionScopes[I]);
+    
+    if (!NextBlock)
+      continue;
+    
     // If we found the defining block for the variable, don't mark the block as
     // having a reference outside it.
     if (NextBlock->TheDecl == VD->getDeclContext())
@@ -1597,7 +1601,8 @@ Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
   // We do not do this for things like enum constants, global variables, etc,
   // as they do not get snapshotted.
   //
-  if (CurBlock && ShouldSnapshotBlockValueReference(CurBlock, VD)) {
+  if (getCurBlock() && 
+      ShouldSnapshotBlockValueReference(*this, getCurBlock(), VD)) {
     if (VD->getType().getTypePtr()->isVariablyModifiedType()) {
       Diag(Loc, diag::err_ref_vm_type);
       Diag(D->getLocation(), diag::note_declared_at);
@@ -6722,29 +6727,15 @@ Sema::OwningExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc,
 
 /// ActOnBlockStart - This callback is invoked when a block literal is started.
 void Sema::ActOnBlockStart(SourceLocation CaretLoc, Scope *BlockScope) {
-  // Analyze block parameters.
+  BlockDecl *Block = BlockDecl::Create(Context, CurContext, CaretLoc);
-  BlockScopeInfo *BSI = new BlockScopeInfo();
+  PushBlockScope(BlockScope, Block);
-
+  CurContext->addDecl(Block);
-  // Add BSI to CurBlock.
+  PushDeclContext(BlockScope, Block);
-  BSI->PrevBlockInfo = CurBlock;
-  CurBlock = BSI;
-
-  BSI->ReturnType = QualType();
-  BSI->TheScope = BlockScope;
-  BSI->hasBlockDeclRefExprs = false;
-  BSI->hasPrototype = false;
-  BSI->SavedFunctionNeedsScopeChecking = CurFunctionNeedsScopeChecking;
-  BSI->SavedNumErrorsAtStartOfFunction = NumErrorsAtStartOfFunction;
-  CurFunctionNeedsScopeChecking = false;
-  NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors();
-  
-  BSI->TheDecl = BlockDecl::Create(Context, CurContext, CaretLoc);
-  CurContext->addDecl(BSI->TheDecl);
-  PushDeclContext(BlockScope, BSI->TheDecl);
 }
 
 void Sema::ActOnBlockArguments(Declarator &ParamInfo, Scope *CurScope) {
   assert(ParamInfo.getIdentifier()==0 && "block-id should have no identifier!");
+  BlockScopeInfo *CurBlock = getCurBlock();
   
   if (ParamInfo.getNumTypeObjects() == 0
       || ParamInfo.getTypeObject(0).Kind != DeclaratorChunk::Function) {
@@ -6847,15 +6838,9 @@ void Sema::ActOnBlockArguments(Declarator &ParamInfo, Scope *CurScope) {
 /// ActOnBlockError - If there is an error parsing a block, this callback
 /// is invoked to pop the information about the block from the action impl.
 void Sema::ActOnBlockError(SourceLocation CaretLoc, Scope *CurScope) {
-  // Ensure that CurBlock is deleted.
-  llvm::OwningPtr<BlockScopeInfo> CC(CurBlock);
-
-  CurFunctionNeedsScopeChecking = CurBlock->SavedFunctionNeedsScopeChecking;
-  NumErrorsAtStartOfFunction = CurBlock->SavedNumErrorsAtStartOfFunction;
-
   // Pop off CurBlock, handle nested blocks.
   PopDeclContext();
-  CurBlock = CurBlock->PrevBlockInfo;
+  PopFunctionOrBlockScope();
   // FIXME: Delete the ParmVarDecl objects as well???
 }
 
@@ -6867,14 +6852,10 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
   if (!LangOpts.Blocks)
     Diag(CaretLoc, diag::err_blocks_disable);
 
-  // Ensure that CurBlock is deleted.
+  BlockScopeInfo *BSI = cast<BlockScopeInfo>(FunctionScopes.back());
-  llvm::OwningPtr<BlockScopeInfo> BSI(CurBlock);
 
   PopDeclContext();
 
-  // Pop off CurBlock, handle nested blocks.
-  CurBlock = CurBlock->PrevBlockInfo;
-
   QualType RetTy = Context.VoidTy;
   if (!BSI->ReturnType.isNull())
     RetTy = BSI->ReturnType;
@@ -6898,13 +6879,9 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
   BlockTy = Context.getBlockPointerType(BlockTy);
 
   // If needed, diagnose invalid gotos and switches in the block.
-  if (CurFunctionNeedsScopeChecking &&
+  if (FunctionNeedsScopeChecking() && !hasAnyErrorsInThisFunction())
-      NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors())
     DiagnoseInvalidJumps(static_cast<CompoundStmt*>(body.get()));
 
-  CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking;
-  NumErrorsAtStartOfFunction = BSI->SavedNumErrorsAtStartOfFunction;
-
   BSI->TheDecl->setBody(body.takeAs<CompoundStmt>());
 
   bool Good = true;
@@ -6922,15 +6899,18 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
     Diag(L->getIdentLoc(), diag::err_undeclared_label_use) << L->getName();
     Good = false;
   }
-  BSI->LabelMap.clear();
+  if (!Good) {
-  if (!Good)
+    PopFunctionOrBlockScope();
     return ExprError();
+  }
   
   AnalysisContext AC(BSI->TheDecl);
   CheckFallThroughForBlock(BlockTy, BSI->TheDecl->getBody(), AC);
   CheckUnreachable(AC);
-  return Owned(new (Context) BlockExpr(BSI->TheDecl, BlockTy,
+  Expr *Result = new (Context) BlockExpr(BSI->TheDecl, BlockTy,
-                                       BSI->hasBlockDeclRefExprs));
+                                         BSI->hasBlockDeclRefExprs);
+  PopFunctionOrBlockScope();  
+  return Owned(Result);
 }
 
 Sema::OwningExprResult Sema::ActOnVAArg(SourceLocation BuiltinLoc,

clang/lib/Sema/SemaStmt.cpp

@@ -1036,6 +1036,7 @@ Action::OwningStmtResult
 Sema::ActOnBlockReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
   // If this is the first return we've seen in the block, infer the type of
   // the block from it.
+  BlockScopeInfo *CurBlock = getCurBlock();
   if (CurBlock->ReturnType.isNull()) {
     if (RetValExp) {
       // Don't call UsualUnaryConversions(), since we don't want to do
@@ -1130,7 +1131,7 @@ static bool IsReturnCopyElidable(ASTContext &Ctx, QualType RetType,
 Action::OwningStmtResult
 Sema::ActOnReturnStmt(SourceLocation ReturnLoc, ExprArg rex) {
   Expr *RetValExp = rex.takeAs<Expr>();
-  if (CurBlock)
+  if (getCurBlock())
     return ActOnBlockReturnStmt(ReturnLoc, RetValExp);
 
   QualType FnRetType;
@@ -1500,7 +1501,7 @@ Sema::ActOnObjCAtFinallyStmt(SourceLocation AtLoc, StmtArg Body) {
 Action::OwningStmtResult
 Sema::ActOnObjCAtTryStmt(SourceLocation AtLoc,
                          StmtArg Try, StmtArg Catch, StmtArg Finally) {
-  CurFunctionNeedsScopeChecking = true;
+  FunctionNeedsScopeChecking() = true;
   return Owned(new (Context) ObjCAtTryStmt(AtLoc, Try.takeAs<Stmt>(),
                                            Catch.takeAs<Stmt>(),
                                            Finally.takeAs<Stmt>()));
@@ -1533,7 +1534,7 @@ Sema::ActOnObjCAtThrowStmt(SourceLocation AtLoc, ExprArg expr,Scope *CurScope) {
 Action::OwningStmtResult
 Sema::ActOnObjCAtSynchronizedStmt(SourceLocation AtLoc, ExprArg SynchExpr,
                                   StmtArg SynchBody) {
-  CurFunctionNeedsScopeChecking = true;
+  FunctionNeedsScopeChecking() = true;
 
   // Make sure the expression type is an ObjC pointer or "void *".
   Expr *SyncExpr = static_cast<Expr*>(SynchExpr.get());
@@ -1643,7 +1644,7 @@ Sema::ActOnCXXTryBlock(SourceLocation TryLoc, StmtArg TryBlock,
   // Neither of these are explicitly forbidden, but every compiler detects them
   // and warns.
 
-  CurFunctionNeedsScopeChecking = true;
+  FunctionNeedsScopeChecking() = true;
   RawHandlers.release();
   return Owned(CXXTryStmt::Create(Context, TryLoc,
                                   static_cast<Stmt*>(TryBlock.release()),

clang/test/Sema/warn-unused-variables.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -fblocks -verify %s
 
 struct s0 {
 	unsigned int	i;
@@ -23,3 +23,10 @@ int f2() {
   int X = 4;  // Shouldn't have a bogus 'unused variable X' warning.
   return Y + X; // expected-error {{use of undeclared identifier 'Y'}}
 }
+
+int f3() {
+  int X1 = 4; 
+  (void)(Y1 + X1); // expected-error {{use of undeclared identifier 'Y1'}}
+  (void)(^() { int X = 4; }); // expected-warning{{unused}}
+  (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of undeclared identifier 'Y'}}
+}

clang/test/SemaCXX/local-classes.cpp

@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace PR6382 {
+  int foo()
+  {
+    goto error;
+    {
+      struct BitPacker {
+        BitPacker() {}
+      };
+      BitPacker packer;
+    }
+
+  error:
+    return -1;
+  }
+}