From 9a28e84b325851310ca2cdfa59d8d8f2875d5a2e Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Mon, 1 Mar 2010 23:15:13 +0000 Subject: [PATCH] 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 | 8 ++ clang/lib/Parse/Parser.cpp | 1 + clang/lib/Sema/Sema.cpp | 64 ++++++++++++- clang/lib/Sema/Sema.h | 116 ++++++++++++++---------- clang/lib/Sema/SemaChecking.cpp | 1 + clang/lib/Sema/SemaCodeComplete.cpp | 5 +- clang/lib/Sema/SemaDecl.cpp | 56 +++++++----- clang/lib/Sema/SemaDeclObjC.cpp | 8 +- clang/lib/Sema/SemaExpr.cpp | 72 ++++++--------- clang/lib/Sema/SemaStmt.cpp | 9 +- clang/test/Sema/warn-unused-variables.c | 9 +- clang/test/SemaCXX/local-classes.cpp | 17 ++++ 12 files changed, 230 insertions(+), 136 deletions(-) create mode 100644 clang/test/SemaCXX/local-classes.cpp diff --git a/clang/include/clang/Parse/Scope.h b/clang/include/clang/Parse/Scope.h index c6a1e53472c1..ef1966cfde6d 100644 --- a/clang/include/clang/Parse/Scope.h +++ b/clang/include/clang/Parse/Scope.h @@ -129,6 +129,9 @@ private: typedef llvm::SmallVector 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; } }; diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index a6ae9cfd0d3c..223d8865d617 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/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. diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index fb7d1991fb53..3b4afef70b53 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -26,7 +26,18 @@ #include "clang/Basic/PartialDiagnostic.h" #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(FunctionScopes.back()); +} + void Sema::ActOnComment(SourceRange Comment) { Context.Comments.push_back(Comment); } diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index 2d6fd2629b14..efd04e8eddd0 100644 --- a/clang/lib/Sema/Sema.h +++ b/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,13 +133,17 @@ struct FunctionScopeInfo { /// block. llvm::SmallVector SwitchStack; - /// \brief Whether this scope information structure defined information for - /// a block. - bool IsBlockInfo; + FunctionScopeInfo(unsigned NumErrors) + : IsBlockInfo(false), NeedsScopeChecking(false), + NumErrorsAtStartOfFunction(NumErrors) { } + + virtual ~FunctionScopeInfo(); + + /// \brief Clear out the information in this function scope, making it + /// suitable for reuse. + void Clear(unsigned NumErrors); - FunctionScopeInfo() : IsBlockInfo(false) { } - - static bool classof(const FunctionScopeInfo *FSI) { return true; } + static bool classof(const FunctionScopeInfo *FSI) { return true; } }; @@ -147,18 +164,15 @@ struct BlockScopeInfo : FunctionScopeInfo { /// return types, if any, in the block body. QualType ReturnType; - /// SavedNumErrorsAtStartOfFunction - This is the value of the - /// NumErrorsAtStartOfFunction variable at the point when the block started. - unsigned SavedNumErrorsAtStartOfFunction; - - /// SavedFunctionNeedsScopeChecking - This is the value of - /// CurFunctionNeedsScopeChecking at the point when the block started. - bool SavedFunctionNeedsScopeChecking; + BlockScopeInfo(unsigned NumErrors, Scope *BlockScope, BlockDecl *Block) + : FunctionScopeInfo(NumErrors), hasPrototype(false), isVariadic(false), + hasBlockDeclRefExprs(false), TheDecl(Block), TheScope(BlockScope) + { + IsBlockInfo = true; + } + + virtual ~BlockScopeInfo(); - 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 - /// 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. + /// \brief Stack containing information about each of the nested function, + /// block, and method scopes that are currently active. + llvm::SmallVector FunctionScopes; + + /// \brief Cached function scope object used for the top function scope + /// and when there is no function scope (in error cases). /// - /// Note that this should always be accessed through getLabelMap() in order - /// to handle blocks properly. - llvm::DenseMap FunctionLabelMap; - - /// FunctionSwitchStack - This is the current set of active switch statements - /// in the top level function. Clients should always use getSwitchStack() to - /// handle the case when they are in a block. - llvm::SmallVector 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 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 &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 &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 &WeakTopLevelDecls() { return WeakTopLevelDecl; } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index c7b9cb7b9bef..30a6ab465538 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/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; diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index b8715c0cbc1a..edf1bc51eb92 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -1137,8 +1137,9 @@ static void AddOrdinaryNameResults(Action::CodeCompletionContext CCC, else if (ObjCMethodDecl *Method = dyn_cast(SemaRef.CurContext)) isVoid = Method->getResultType()->isVoidType(); - else if (SemaRef.CurBlock && !SemaRef.CurBlock->ReturnType.isNull()) - isVoid = SemaRef.CurBlock->ReturnType->isVoidType(); + else if (SemaRef.getCurBlock() && + !SemaRef.getCurBlock()->ReturnType.isNull()) + isVoid = SemaRef.getCurBlock()->ReturnType->isVoidType(); Pattern = new CodeCompletionString; Pattern->AddTypedTextChunk("return"); if (!isVoid) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 1d448d0de825..d7cbb8fd59df 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -553,8 +553,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) { if (!D->getDeclName()) continue; // Diagnose unused variables in this scope. - if (ShouldDiagnoseUnusedDecl(D) && - NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) + if (ShouldDiagnoseUnusedDecl(D) && + 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(D.getAs()); - CurFunctionNeedsScopeChecking = false; - NumErrorsAtStartOfFunction = getDiagnostics().getNumErrors(); + // Enter a new function scope + 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::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,25 +4255,34 @@ 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; - - CheckUnreachable(AC); + if (Body) { + CheckUnreachable(AC); + // 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(FD) && isa(Body)) + DiagnoseReturnInConstructorExceptionHandler(cast(Body)); + // Verify that that gotos and switch cases don't jump into scopes illegally. - if (CurFunctionNeedsScopeChecking && - NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) - DiagnoseInvalidJumps(Body); + // Verify that that gotos and switch cases don't jump into scopes illegally. + if (FunctionNeedsScopeChecking() && !hasAnyErrorsInThisFunction()) + 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(FD) && isa(Body)) - DiagnoseReturnInConstructorExceptionHandler(cast(Body)); - - if (CXXDestructorDecl *Destructor = dyn_cast(dcl)) - MarkBaseAndMemberDestructorsReferenced(Destructor); + if (CXXDestructorDecl *Destructor = dyn_cast(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 @@ -4283,7 +4290,6 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, if (getDiagnostics().hasErrorOccurred()) ExprTemporaries.clear(); - assert(ExprTemporaries.empty() && "Leftover temporaries in function"); return D; } diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp index 0ec793360b05..149fe15fabec 100644 --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/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,7 +57,8 @@ 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; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 966cbc72a254..d9464ad8c009 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/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; - NextBlock = NextBlock->PrevBlockInfo) { + for (unsigned I = S.FunctionScopes.size() - 1; I; --I) { + BlockScopeInfo *NextBlock = dyn_cast(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,30 +6727,16 @@ 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. - BlockScopeInfo *BSI = new BlockScopeInfo(); - - // Add BSI to CurBlock. - 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); + BlockDecl *Block = BlockDecl::Create(Context, CurContext, CaretLoc); + PushBlockScope(BlockScope, Block); + CurContext->addDecl(Block); + PushDeclContext(BlockScope, Block); } 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) { ProcessDeclAttributes(CurScope, CurBlock->TheDecl, ParamInfo); @@ -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 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. - llvm::OwningPtr BSI(CurBlock); + BlockScopeInfo *BSI = cast(FunctionScopes.back()); PopDeclContext(); - // Pop off CurBlock, handle nested blocks. - CurBlock = CurBlock->PrevBlockInfo; - QualType RetTy = Context.VoidTy; if (!BSI->ReturnType.isNull()) RetTy = BSI->ReturnType; @@ -6898,12 +6879,8 @@ Sema::OwningExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc, BlockTy = Context.getBlockPointerType(BlockTy); // If needed, diagnose invalid gotos and switches in the block. - if (CurFunctionNeedsScopeChecking && - NumErrorsAtStartOfFunction == getDiagnostics().getNumErrors()) + if (FunctionNeedsScopeChecking() && !hasAnyErrorsInThisFunction()) DiagnoseInvalidJumps(static_cast(body.get())); - - CurFunctionNeedsScopeChecking = BSI->SavedFunctionNeedsScopeChecking; - NumErrorsAtStartOfFunction = BSI->SavedNumErrorsAtStartOfFunction; BSI->TheDecl->setBody(body.takeAs()); @@ -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, - BSI->hasBlockDeclRefExprs)); + Expr *Result = new (Context) BlockExpr(BSI->TheDecl, BlockTy, + BSI->hasBlockDeclRefExprs); + PopFunctionOrBlockScope(); + return Owned(Result); } Sema::OwningExprResult Sema::ActOnVAArg(SourceLocation BuiltinLoc, diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 540189cd8307..75d9f67ad9d2 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/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(); - 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(), Catch.takeAs(), Finally.takeAs())); @@ -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(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(TryBlock.release()), diff --git a/clang/test/Sema/warn-unused-variables.c b/clang/test/Sema/warn-unused-variables.c index 4d1cde7067e5..58e52b171da5 100644 --- a/clang/test/Sema/warn-unused-variables.c +++ b/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'}} +} diff --git a/clang/test/SemaCXX/local-classes.cpp b/clang/test/SemaCXX/local-classes.cpp new file mode 100644 index 000000000000..3c216d0863e2 --- /dev/null +++ b/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; + } +}