[cfe-commits] r133610 - in /cfe/trunk: include/clang/Basic/DelayedCleanupPool.h include/clang/Parse/Parser.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseExpr.cpp lib/Parse/ParseExprCXX.cpp lib/Parse/ParseTemplate.cpp lib/Parse/ParseTentative.cpp lib/Parse/Parser.cpp lib/Sema/DeclSpec.cpp
Douglas Gregor
dgregor at apple.com
Wed Jun 22 07:04:27 PDT 2011
On Jun 21, 2011, at 11:09 PM, Argyrios Kyrtzidis wrote:
> Author: akirtzidis
> Date: Wed Jun 22 01:09:49 2011
> New Revision: 133610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=133610&view=rev
> Log:
> Introduce DelayedCleanupPool useful for simplifying clean-up of certain resources that, while their
> lifetime is well-known and restricted, cleaning them up manually is easy to miss and cause a leak.
>
> Use it to plug the leaking of TemplateIdAnnotation objects. rdar://9634138.
Very nice, thanks!
- Doug
> Added:
> cfe/trunk/include/clang/Basic/DelayedCleanupPool.h
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> cfe/trunk/lib/Parse/ParseExpr.cpp
> cfe/trunk/lib/Parse/ParseExprCXX.cpp
> cfe/trunk/lib/Parse/ParseTemplate.cpp
> cfe/trunk/lib/Parse/ParseTentative.cpp
> cfe/trunk/lib/Parse/Parser.cpp
> cfe/trunk/lib/Sema/DeclSpec.cpp
>
> Added: cfe/trunk/include/clang/Basic/DelayedCleanupPool.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DelayedCleanupPool.h?rev=133610&view=auto
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DelayedCleanupPool.h (added)
> +++ cfe/trunk/include/clang/Basic/DelayedCleanupPool.h Wed Jun 22 01:09:49 2011
> @@ -0,0 +1,109 @@
> +//=== DelayedCleanupPool.h - Delayed Clean-up Pool Implementation *- C++ -*===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file defines a facility to delay calling cleanup methods until specific
> +// points.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_CLANG_BASIC_DELAYEDCLEANUPPOOL_H
> +#define LLVM_CLANG_BASIC_DELAYEDCLEANUPPOOL_H
> +
> +#include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/SmallVector.h"
> +
> +namespace clang {
> +
> +/// \brief Gathers pairs of pointer-to-object/pointer-to-cleanup-function
> +/// allowing the cleanup functions to get called (with the pointer as parameter)
> +/// at specific points.
> +///
> +/// The use case is to simplify clean-up of certain resources that, while their
> +/// lifetime is well-known and restricted, cleaning them up manually is easy to
> +/// miss and cause a leak.
> +///
> +/// The same pointer can be added multiple times; its clean-up function will
> +/// only be called once.
> +class DelayedCleanupPool {
> +public:
> + typedef void (*CleanupFn)(void *ptr);
> +
> + /// \brief Adds a pointer and its associated cleanup function to be called
> + /// at a later point.
> + ///
> + /// \returns false if the pointer is already added, true otherwise.
> + bool delayCleanup(void *ptr, CleanupFn fn) {
> + assert(ptr && "Expected valid pointer to object");
> + assert(fn && "Expected valid pointer to function");
> +
> + CleanupFn &mapFn = Ptrs[ptr];
> + assert((!mapFn || mapFn == fn) &&
> + "Adding a pointer with different cleanup function!");
> +
> + if (!mapFn) {
> + mapFn = fn;
> + Cleanups.push_back(std::make_pair(ptr, fn));
> + return true;
> + }
> +
> + return false;
> + }
> +
> + template <typename T>
> + bool delayDelete(T *ptr) {
> + return delayCleanup(ptr, cleanupWithDelete<T>);
> + }
> +
> + template <typename T, void (T::*Fn)()>
> + bool delayMemberFunc(T *ptr) {
> + return delayCleanup(ptr, cleanupWithMemberFunc<T, Fn>);
> + }
> +
> + void doCleanup() {
> + for (llvm::SmallVector<std::pair<void *, CleanupFn>, 8>::reverse_iterator
> + I = Cleanups.rbegin(), E = Cleanups.rend(); I != E; ++I)
> + I->second(I->first);
> + Cleanups.clear();
> + Ptrs.clear();
> + }
> +
> + ~DelayedCleanupPool() {
> + doCleanup();
> + }
> +
> +private:
> + llvm::DenseMap<void *, CleanupFn> Ptrs;
> + llvm::SmallVector<std::pair<void *, CleanupFn>, 8> Cleanups;
> +
> + template <typename T>
> + static void cleanupWithDelete(void *ptr) {
> + delete static_cast<T *>(ptr);
> + }
> +
> + template <typename T, void (T::*Fn)()>
> + static void cleanupWithMemberFunc(void *ptr) {
> + (static_cast<T *>(ptr)->*Fn)();
> + }
> +};
> +
> +/// \brief RAII object for triggering a cleanup of a DelayedCleanupPool.
> +class DelayedCleanupPoint {
> + DelayedCleanupPool &Pool;
> +
> +public:
> + DelayedCleanupPoint(DelayedCleanupPool &pool) : Pool(pool) { }
> +
> + ~DelayedCleanupPoint() {
> + Pool.doCleanup();
> + }
> +};
> +
> +} // end namespace clang
> +
> +#endif
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Wed Jun 22 01:09:49 2011
> @@ -15,6 +15,7 @@
> #define LLVM_CLANG_PARSE_PARSER_H
>
> #include "clang/Basic/Specifiers.h"
> +#include "clang/Basic/DelayedCleanupPool.h"
> #include "clang/Lex/Preprocessor.h"
> #include "clang/Lex/CodeCompletionHandler.h"
> #include "clang/Sema/Sema.h"
> @@ -170,6 +171,10 @@
> /// Factory object for creating AttributeList objects.
> AttributeFactory AttrFactory;
>
> + /// \brief Gathers and cleans up objects when parsing of a top-level
> + /// declaration is finished.
> + DelayedCleanupPool TopLevelDeclCleanupPool;
> +
> public:
> Parser(Preprocessor &PP, Sema &Actions);
> ~Parser();
> @@ -467,7 +472,12 @@
> bool TryAltiVecTokenOutOfLine(DeclSpec &DS, SourceLocation Loc,
> const char *&PrevSpec, unsigned &DiagID,
> bool &isInvalid);
> -
> +
> + /// \brief Get the TemplateIdAnnotation from the token and put it in the
> + /// cleanup pool so that it gets destroyed when parsing the current top level
> + /// declaration is finished.
> + TemplateIdAnnotation *takeTemplateIdAnnotation(const Token &tok);
> +
> /// TentativeParsingAction - An object that is used as a kind of "tentative
> /// parsing transaction". It gets instantiated to mark the token position and
> /// after the token consumption is done, Commit() or Revert() is called to
>
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Jun 22 01:09:49 2011
> @@ -1396,8 +1396,7 @@
> // Thus, if the template-name is actually the constructor
> // name, then the code is ill-formed; this interpretation is
> // reinforced by the NAD status of core issue 635.
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Next.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Next);
> if ((DSContext == DSC_top_level ||
> (DSContext == DSC_class && DS.isFriendSpecified())) &&
> TemplateId->Name &&
> @@ -1599,8 +1598,7 @@
>
> // type-name
> case tok::annot_template_id: {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind != TNK_Type_template) {
> // This template-id does not refer to a type name, so we're
> // done with the type-specifiers.
>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Wed Jun 22 01:09:49 2011
> @@ -701,8 +701,7 @@
> CXXScopeSpec &SS) {
> // Check whether we have a template-id that names a type.
> if (Tok.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind == TNK_Type_template ||
> TemplateId->Kind == TNK_Dependent_template_name) {
> AnnotateTemplateIdTokenAsType();
> @@ -976,7 +975,7 @@
> }
> }
> } else if (Tok.is(tok::annot_template_id)) {
> - TemplateId = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateId = takeTemplateIdAnnotation(Tok);
> NameLoc = ConsumeToken();
>
> if (TemplateId->Kind != TNK_Type_template &&
> @@ -993,7 +992,6 @@
>
> DS.SetTypeSpecError();
> SkipUntil(tok::semi, false, true);
> - TemplateId->Destroy();
> if (SuppressingAccessChecks)
> Actions.ActOnStopSuppressingAccessChecks();
>
> @@ -1051,9 +1049,6 @@
> }
>
> SkipUntil(tok::comma, true);
> -
> - if (TemplateId)
> - TemplateId->Destroy();
> return;
> }
>
> @@ -1149,7 +1144,6 @@
> TemplateParams? &(*TemplateParams)[0] : 0,
> TemplateParams? TemplateParams->size() : 0));
> }
> - TemplateId->Destroy();
> } else if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation &&
> TUK == Sema::TUK_Declaration) {
> // Explicit instantiation of a member of a class template
> @@ -2248,8 +2242,7 @@
> ParseOptionalCXXScopeSpecifier(SS, ParsedType(), false);
> ParsedType TemplateTypeTy;
> if (Tok.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind == TNK_Type_template ||
> TemplateId->Kind == TNK_Dependent_template_name) {
> AnnotateTemplateIdTokenAsType();
>
> Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Jun 22 01:09:49 2011
> @@ -955,8 +955,7 @@
>
> Token Next = NextToken();
> if (Next.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Next.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Next);
> if (TemplateId->Kind == TNK_Type_template) {
> // We have a qualified template-id that we know refers to a
> // type, translate it into a type and continue parsing as a
> @@ -975,8 +974,7 @@
> }
>
> case tok::annot_template_id: { // [C++] template-id
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind == TNK_Type_template) {
> // We have a template-id that we know refers to a type,
> // translate it into a type and continue parsing as a cast
>
> Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Wed Jun 22 01:09:49 2011
> @@ -245,8 +245,7 @@
> // So we need to check whether the simple-template-id is of the
> // right kind (it should name a type or be dependent), and then
> // convert it into a type within the nested-name-specifier.
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (CheckForDestructor && GetLookAheadToken(2).is(tok::tilde)) {
> *MayBePseudoDestructor = true;
> return false;
> @@ -281,10 +280,6 @@
> SS.SetInvalid(SourceRange(StartLoc, CCLoc));
> }
>
> - // If we are caching tokens we will process the TemplateId again,
> - // otherwise destroy it.
> - if (!PP.isBacktrackEnabled())
> - TemplateId->Destroy();
> continue;
> }
>
> @@ -1606,8 +1601,7 @@
> // unqualified-id:
> // template-id (already parsed and annotated)
> if (Tok.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation*>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
>
> // If the template-name names the current class, then this is a constructor
> if (AllowConstructorName && TemplateId->Name &&
> @@ -1630,7 +1624,6 @@
> /*NontrivialTypeSourceInfo=*/true),
> TemplateId->TemplateNameLoc,
> TemplateId->RAngleLoc);
> - TemplateId->Destroy();
> ConsumeToken();
> return false;
> }
>
> Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseTemplate.cpp Wed Jun 22 01:09:49 2011
> @@ -861,8 +861,7 @@
> void Parser::AnnotateTemplateIdTokenAsType() {
> assert(Tok.is(tok::annot_template_id) && "Requires template-id tokens");
>
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> assert((TemplateId->Kind == TNK_Type_template ||
> TemplateId->Kind == TNK_Dependent_template_name) &&
> "Only works for type and dependent templates");
> @@ -888,7 +887,6 @@
> // Replace the template-id annotation token, and possible the scope-specifier
> // that precedes it, with the typename annotation token.
> PP.AnnotateCachedTokens(Tok);
> - TemplateId->Destroy();
> }
>
> /// \brief Determine whether the given token can end a template argument.
>
> Modified: cfe/trunk/lib/Parse/ParseTentative.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseTentative.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseTentative.cpp Wed Jun 22 01:09:49 2011
> @@ -908,8 +908,7 @@
> return TPResult::True();
>
> case tok::annot_template_id: {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind != TNK_Type_template)
> return TPResult::False();
> CXXScopeSpec SS;
>
> Modified: cfe/trunk/lib/Parse/Parser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/Parser.cpp (original)
> +++ cfe/trunk/lib/Parse/Parser.cpp Wed Jun 22 01:09:49 2011
> @@ -486,6 +486,7 @@
> /// ParseTopLevelDecl - Parse one top-level declaration, return whatever the
> /// action tells us to. This returns true if the EOF was encountered.
> bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result) {
> + DelayedCleanupPoint CleanupRAII(TopLevelDeclCleanupPool);
>
> while (Tok.is(tok::annot_pragma_unused))
> HandlePragmaUnused();
> @@ -548,6 +549,7 @@
> Parser::DeclGroupPtrTy
> Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
> ParsingDeclSpec *DS) {
> + DelayedCleanupPoint CleanupRAII(TopLevelDeclCleanupPool);
> ParenBraceBracketBalancer BalancerRAIIObj(*this);
>
> Decl *SingleDecl = 0;
> @@ -1155,6 +1157,18 @@
> return move(Result);
> }
>
> +/// \brief Get the TemplateIdAnnotation from the token and put it in the
> +/// cleanup pool so that it gets destroyed when parsing the current top level
> +/// declaration is finished.
> +TemplateIdAnnotation *Parser::takeTemplateIdAnnotation(const Token &tok) {
> + assert(tok.is(tok::annot_template_id) && "Expected template-id token");
> + TemplateIdAnnotation *
> + Id = static_cast<TemplateIdAnnotation *>(tok.getAnnotationValue());
> + TopLevelDeclCleanupPool.delayMemberFunc< TemplateIdAnnotation,
> + &TemplateIdAnnotation::Destroy>(Id);
> + return Id;
> +}
> +
> /// TryAnnotateTypeOrScopeToken - If the current token position is on a
> /// typename (possibly qualified in C++) or a C++ scope specifier not followed
> /// by a typename, TryAnnotateTypeOrScopeToken will replace one or more tokens
> @@ -1209,8 +1223,7 @@
> *Tok.getIdentifierInfo(),
> Tok.getLocation());
> } else if (Tok.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind == TNK_Function_template) {
> Diag(Tok, diag::err_typename_refers_to_non_type_template)
> << Tok.getAnnotationRange();
> @@ -1228,7 +1241,6 @@
> TemplateId->LAngleLoc,
> TemplateArgsPtr,
> TemplateId->RAngleLoc);
> - TemplateId->Destroy();
> } else {
> Diag(Tok, diag::err_expected_type_name_after_typename)
> << SS.getRange();
> @@ -1311,8 +1323,7 @@
> }
>
> if (Tok.is(tok::annot_template_id)) {
> - TemplateIdAnnotation *TemplateId
> - = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
> + TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
> if (TemplateId->Kind == TNK_Type_template) {
> // A template-id that refers to a type was parsed into a
> // template-id annotation in a context where we weren't allowed
>
> Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=133610&r1=133609&r2=133610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
> +++ cfe/trunk/lib/Sema/DeclSpec.cpp Wed Jun 22 01:09:49 2011
> @@ -792,9 +792,6 @@
> }
>
> void UnqualifiedId::clear() {
> - if (Kind == IK_TemplateId)
> - TemplateId->Destroy();
> -
> Kind = IK_Identifier;
> Identifier = 0;
> StartLocation = SourceLocation();
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list