[clang-tools-extra] r213851 - Plug memory leaks.
David Blaikie
dblaikie at gmail.com
Fri Aug 8 09:20:52 PDT 2014
On Fri, Aug 8, 2014 at 9:18 AM, David Blaikie <dblaikie at gmail.com> wrote:
> I simplified some of this by having createActionFactory return
> unique_ptr directly (and removed the local variables your patch
> introduced, in favor of just inline "get()" calls) as well as
> improving ownership of the CompilationDatabase factory functions.
>
> clang: r215215
> clang-tools-extra: r215214
>
> I think the bugs related to passing new ActionFactories directly to
> Tool.run could be reduced by having Tool.run take by reference,
> instead of pointer (I'll bump http://reviews.llvm.org/D4312 with this
> point soon), making it clear that no ownership transfer can happen
> there.
(as I review D4312 I see it actually did fix all the ActionFactory
bugs you caught/fixed here... by replacing ".run(new
FooActionFactory(...))" with ".run(FooActionFactory(...))")
>
> On Thu, Jul 24, 2014 at 3:23 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Thu Jul 24 05:23:33 2014
>> New Revision: 213851
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=213851&view=rev
>> Log:
>> Plug memory leaks.
>>
>> Most of the changes are mechanic std::unique_ptr insertions. All leaks were
>> detected by LeakSanitizer.
>>
>> Modified:
>> clang-tools-extra/trunk/clang-modernize/AddOverride/AddOverride.cpp
>> clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopConvert.cpp
>> clang-tools-extra/trunk/clang-modernize/PassByValue/PassByValue.cpp
>> clang-tools-extra/trunk/clang-modernize/ReplaceAutoPtr/ReplaceAutoPtr.cpp
>> clang-tools-extra/trunk/clang-modernize/UseAuto/UseAuto.cpp
>> clang-tools-extra/trunk/clang-modernize/UseNullptr/UseNullptr.cpp
>> clang-tools-extra/trunk/clang-modernize/tool/ClangModernize.cpp
>> clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
>> clang-tools-extra/trunk/modularize/Modularize.cpp
>> clang-tools-extra/trunk/pp-trace/PPTrace.cpp
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/AddOverride/AddOverride.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/AddOverride/AddOverride.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/AddOverride/AddOverride.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/AddOverride/AddOverride.cpp Thu Jul 24 05:23:33 2014
>> @@ -40,7 +40,8 @@ int AddOverrideTransform::apply(const Co
>> // Make Fixer available to handleBeginSource().
>> this->Fixer = &Fixer;
>>
>> - if (int result = AddOverrideTool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (int result = AddOverrideTool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return result;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopConvert.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopConvert.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopConvert.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/LoopConvert/LoopConvert.cpp Thu Jul 24 05:23:33 2014
>> @@ -48,7 +48,8 @@ int LoopConvertTransform::apply(const Co
>> LFK_PseudoArray, /*Owner=*/ *this);
>> Finder.addMatcher(makePseudoArrayLoopMatcher(), &PseudoarrrayLoopFixer);
>>
>> - if (int result = LoopTool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (int result = LoopTool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return result;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/PassByValue/PassByValue.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/PassByValue/PassByValue.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/PassByValue/PassByValue.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/PassByValue/PassByValue.cpp Thu Jul 24 05:23:33 2014
>> @@ -35,7 +35,8 @@ int PassByValueTransform::apply(const to
>> // make the replacer available to handleBeginSource()
>> this->Replacer = &Replacer;
>>
>> - if (Tool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (Tool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return 1;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/ReplaceAutoPtr/ReplaceAutoPtr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/ReplaceAutoPtr/ReplaceAutoPtr.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/ReplaceAutoPtr/ReplaceAutoPtr.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/ReplaceAutoPtr/ReplaceAutoPtr.cpp Thu Jul 24 05:23:33 2014
>> @@ -34,7 +34,8 @@ ReplaceAutoPtrTransform::apply(const Com
>> Finder.addMatcher(makeAutoPtrUsingDeclMatcher(), &Replacer);
>> Finder.addMatcher(makeTransferOwnershipExprMatcher(), &Fixer);
>>
>> - if (Tool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (Tool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return 1;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/UseAuto/UseAuto.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/UseAuto/UseAuto.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/UseAuto/UseAuto.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/UseAuto/UseAuto.cpp Thu Jul 24 05:23:33 2014
>> @@ -36,7 +36,8 @@ int UseAutoTransform::apply(const clang:
>> Finder.addMatcher(makeIteratorDeclMatcher(), &ReplaceIterators);
>> Finder.addMatcher(makeDeclWithNewMatcher(), &ReplaceNew);
>>
>> - if (int Result = UseAutoTool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (int Result = UseAutoTool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return Result;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/UseNullptr/UseNullptr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/UseNullptr/UseNullptr.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/UseNullptr/UseNullptr.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/UseNullptr/UseNullptr.cpp Thu Jul 24 05:23:33 2014
>> @@ -46,8 +46,8 @@ int UseNullptrTransform::apply(const Com
>> NullptrFixer Fixer(AcceptedChanges, MacroNames, /*Owner=*/ *this);
>>
>> Finder.addMatcher(makeCastSequenceMatcher(), &Fixer);
>> -
>> - if (int result = UseNullptrTool.run(createActionFactory(Finder))) {
>> + std::unique_ptr<FrontendActionFactory> Factory(createActionFactory(Finder));
>> + if (int result = UseNullptrTool.run(Factory.get())) {
>> llvm::errs() << "Error encountered during translation.\n";
>> return result;
>> }
>>
>> Modified: clang-tools-extra/trunk/clang-modernize/tool/ClangModernize.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/tool/ClangModernize.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-modernize/tool/ClangModernize.cpp (original)
>> +++ clang-tools-extra/trunk/clang-modernize/tool/ClangModernize.cpp Thu Jul 24 05:23:33 2014
>> @@ -255,9 +255,10 @@ CompilationDatabase *autoDetectCompilati
>> ErrorMessage);
>> // Try to auto-detect a compilation database from the first source.
>> if (!SourcePaths.empty()) {
>> - if (CompilationDatabase *Compilations =
>> - CompilationDatabase::autoDetectFromSource(SourcePaths[0],
>> - ErrorMessage)) {
>> + std::unique_ptr<CompilationDatabase> Compilations(
>> + CompilationDatabase::autoDetectFromSource(SourcePaths[0],
>> + ErrorMessage));
>> + if (Compilations) {
>> // FIXME: just pass SourcePaths[0] once getCompileCommands supports
>> // non-absolute paths.
>> SmallString<64> Path(SourcePaths[0]);
>> @@ -267,7 +268,7 @@ CompilationDatabase *autoDetectCompilati
>> // Ignore a detected compilation database that doesn't contain source0
>> // since it is probably an unrelated compilation database.
>> if (!Commands.empty())
>> - return Compilations;
>> + return Compilations.release();
>> }
>> // Reset ErrorMessage since a fix compilation database will be created if
>> // it fails to detect one from source.
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
>> +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu Jul 24 05:23:33 2014
>> @@ -331,9 +331,8 @@ ClangTidyStats runClangTidy(ClangTidyOpt
>>
>> class ActionFactory : public FrontendActionFactory {
>> public:
>> - ActionFactory(ClangTidyASTConsumerFactory *ConsumerFactory)
>> - : ConsumerFactory(ConsumerFactory) {}
>> - FrontendAction *create() override { return new Action(ConsumerFactory); }
>> + ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
>> + FrontendAction *create() override { return new Action(&ConsumerFactory); }
>>
>> private:
>> class Action : public ASTFrontendAction {
>> @@ -348,10 +347,11 @@ ClangTidyStats runClangTidy(ClangTidyOpt
>> ClangTidyASTConsumerFactory *Factory;
>> };
>>
>> - ClangTidyASTConsumerFactory *ConsumerFactory;
>> + ClangTidyASTConsumerFactory ConsumerFactory;
>> };
>>
>> - Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context)));
>> + ActionFactory Factory(Context);
>> + Tool.run(&Factory);
>> *Errors = Context.getErrors();
>> return Context.getStats();
>> }
>>
>> Modified: clang-tools-extra/trunk/modularize/Modularize.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/modularize/Modularize.cpp (original)
>> +++ clang-tools-extra/trunk/modularize/Modularize.cpp Thu Jul 24 05:23:33 2014
>> @@ -736,8 +736,8 @@ int main(int Argc, const char **Argv) {
>> ClangTool Tool(*Compilations, Headers);
>> Tool.appendArgumentsAdjuster(new AddDependenciesAdjuster(Dependencies));
>> int HadErrors = 0;
>> - HadErrors |= Tool.run(
>> - new ModularizeFrontendActionFactory(Entities, *PPTracker, HadErrors));
>> + ModularizeFrontendActionFactory Factory(Entities, *PPTracker, HadErrors);
>> + HadErrors |= Tool.run(&Factory);
>>
>> // Create a place to save duplicate entity locations, separate bins per kind.
>> typedef SmallVector<Location, 8> LocationArray;
>>
>> Modified: clang-tools-extra/trunk/pp-trace/PPTrace.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/pp-trace/PPTrace.cpp?rev=213851&r1=213850&r2=213851&view=diff
>> ==============================================================================
>> --- clang-tools-extra/trunk/pp-trace/PPTrace.cpp (original)
>> +++ clang-tools-extra/trunk/pp-trace/PPTrace.cpp Thu Jul 24 05:23:33 2014
>> @@ -199,8 +199,8 @@ int main(int Argc, const char **Argv) {
>>
>> // Create the tool and run the compilation.
>> ClangTool Tool(*Compilations, SourcePaths);
>> - int HadErrors =
>> - Tool.run(new PPTraceFrontendActionFactory(Ignore, CallbackCalls));
>> + PPTraceFrontendActionFactory Factory(Ignore, CallbackCalls);
>> + int HadErrors = Tool.run(&Factory);
>>
>> // If we had errors, exit early.
>> if (HadErrors)
>>
>>
>> _______________________________________________
>> 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