is it possible to test it? Even if the test only fails when run in a leak checker?
On 3 May 2013 05:15, Manuel Klimek <[email protected]> wrote: > This patch ensures that APValues are deallocated with the ASTContext > by registering a deallocation function for APValues to the ASTContext. > > This patch was written by James Dennett, and I'm taking it over > as this currently provides problems for tools. > > http://llvm-reviews.chandlerc.com/D736 > > Files: > include/clang/AST/ASTContext.h > lib/AST/ASTContext.cpp > lib/AST/Decl.cpp > > Index: include/clang/AST/ASTContext.h > =================================================================== > --- include/clang/AST/ASTContext.h > +++ include/clang/AST/ASTContext.h > @@ -2186,10 +2186,12 @@ > const ObjCImplementationDecl *Impl) const; > > private: > - /// \brief A set of deallocations that should be performed when the > + /// \brief A set of deallocations that should be performed when the > /// ASTContext is destroyed. > - SmallVector<std::pair<void (*)(void*), void *>, 16> Deallocations; > - > + typedef llvm::SmallDenseMap<void(*)(void*), llvm::SmallVector<void*, 16> > > + DeallocationMap; > + DeallocationMap Deallocations; > + > // FIXME: This currently contains the set of StoredDeclMaps used > // by DeclContext objects. This probably should not be in ASTContext, > // but we include it here so that ASTContext can quickly deallocate them. > Index: lib/AST/ASTContext.cpp > =================================================================== > --- lib/AST/ASTContext.cpp > +++ lib/AST/ASTContext.cpp > @@ -726,10 +726,12 @@ > // FIXME: Is this the ideal solution? > ReleaseDeclContextMaps(); > > - // Call all of the deallocation functions. > - for (unsigned I = 0, N = Deallocations.size(); I != N; ++I) > - Deallocations[I].first(Deallocations[I].second); > - > + // Call all of the deallocation functions on all of their targets. > + for (DeallocationMap::const_iterator I = Deallocations.begin(), > + E = Deallocations.end(); I != E; ++I) > + for (unsigned J = 0, N = I->second.size(); J != N; ++J) > + (I->first)((I->second)[J]); > + > // ASTRecordLayout objects in ASTRecordLayouts must always be destroyed > // because they can contain DenseMaps. > for (llvm::DenseMap<const ObjCContainerDecl*, > @@ -753,7 +755,16 @@ > } > > void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) { > - Deallocations.push_back(std::make_pair(Callback, Data)); > + // TODO(jdennett): Optimize the naive code to avoid temporary vectors > + // for the common case where the Callback is already present. The > + // assumption is that few distinct callbacks are used. > + // SmallDenseMap<void (*)(void*), void*>::const_iterator I = > + // Deallocations.find(Callback); > + // if (I == Deallocations.end()) > + // Deallocations[Callback].push_back(Data); > + // else > + // I->second.push_back(Data); > + Deallocations[Callback].push_back(Data); > } > > void > Index: lib/AST/Decl.cpp > =================================================================== > --- lib/AST/Decl.cpp > +++ lib/AST/Decl.cpp > @@ -1742,6 +1742,10 @@ > EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>(); > if (!Eval) { > Stmt *S = Init.get<Stmt *>(); > + // Note: EvaluatedStmt contains an APValue, which usually holds > + // resources not allocated from the ASTContext. We need to do some > + // work to avoid leaking those, but we do so in VarDecl::evaluateValue > + // where we can detect whether there's anything to clean up or not. > Eval = new (getASTContext()) EvaluatedStmt; > Eval->Value = S; > Init = Eval; > @@ -1754,6 +1758,13 @@ > return evaluateValue(Notes); > } > > +namespace { > +// Destroy an APValue that was allocated in an ASTContext. > +void DestroyAPValue(void* UntypedValue) { > + static_cast<APValue*>(UntypedValue)->~APValue(); > +} > +} // namespace > + > APValue *VarDecl::evaluateValue( > SmallVectorImpl<PartialDiagnosticAt> &Notes) const { > EvaluatedStmt *Eval = ensureEvaluatedStmt(); > @@ -1779,8 +1790,12 @@ > bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, getASTContext(), > this, Notes); > > - // Ensure the result is an uninitialized APValue if evaluation fails. > - if (!Result) > + // Ensure the computed APValue is cleaned up later if evaluation succeeded, > + // or that it's empty (so that there's nothing to clean up) if evaluation > + // failed. > + if (Result) > + getASTContext().AddDeallocation(DestroyAPValue, &Eval->Evaluated); > + else > Eval->Evaluated = APValue(); > > Eval->IsEvaluating = false; > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
