[clang-tools-extra] r213851 - Plug memory leaks.

David Blaikie dblaikie at gmail.com
Fri Aug 8 09:18:48 PDT 2014


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.

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