[llvm] r255831 - Simplify memory management with std::unique_ptr.

Wed Dec 16 21:20:01 PST 2015

On Thu, Dec 17, 2015 at 9:28 AM, Rafael Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rafael
> Date: Wed Dec 16 16:28:34 2015
> New Revision: 255831
>
> URL: http://llvm.org/viewvc/llvm-project?rev=255831&view=rev
> Log:
> Simplify memory management with std::unique_ptr.
>
> Modified:
>     llvm/trunk/include/llvm/ADT/Statistic.h
>     llvm/trunk/lib/Support/Statistic.cpp
>     llvm/trunk/lib/Support/Timer.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/Statistic.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Statistic.h?rev=255831&r1=255830&r2=255831&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/Statistic.h (original)
> +++ llvm/trunk/include/llvm/ADT/Statistic.h Wed Dec 16 16:28:34 2015
> @@ -28,9 +28,11 @@
>
>  #include "llvm/Support/Atomic.h"
>  #include "llvm/Support/Valgrind.h"
> +#include <memory>
>
>  namespace llvm {
>  class raw_ostream;
> +class raw_fd_ostream;
>
>  class Statistic {
>  public:
> @@ -170,6 +172,9 @@ void EnableStatistics();
>  /// \brief Check if statistics are enabled.
>  bool AreStatisticsEnabled();
>
> +/// \brief Return a file stream to print our output on.
> +std::unique_ptr<raw_fd_ostream> CreateInfoOutputFile();
> +
>  /// \brief Print statistics to the file returned by
> CreateInfoOutputFile().
>  void PrintStatistics();
>
>
> Modified: llvm/trunk/lib/Support/Statistic.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Statistic.cpp?rev=255831&r1=255830&r2=255831&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/Statistic.cpp (original)
> +++ llvm/trunk/lib/Support/Statistic.cpp Wed Dec 16 16:28:34 2015
> @@ -34,9 +34,6 @@
>  #include <cstring>
>  using namespace llvm;
>
> -// CreateInfoOutputFile - Return a file stream to print our output on.
> -namespace llvm { extern raw_ostream *CreateInfoOutputFile(); }
> -
>  /// -stats - Command line option to cause transformations to emit stats
> about
>  /// what they did.
>  ///
> @@ -145,20 +142,18 @@ void llvm::PrintStatistics() {
>    if (Stats.Stats.empty()) return;
>
>    // Get the stream to write to.
> -  raw_ostream &OutStream = *CreateInfoOutputFile();
> -  PrintStatistics(OutStream);
> -  delete &OutStream;   // Close the file.
> +  std::unique_ptr<raw_ostream> OutStream = CreateInfoOutputFile();
> +  PrintStatistics(*OutStream);
>

Looks like now that this doesn't need explicit cleanup, you could remove
the variable, but that might be a bit unclear:

PrintStatistics(*CreateInfoOutputFile());

Can (or is it already) raw_ostream be made movable? In which case
CretaeInfoInputFile could return by value? (but I guess then you couldn't
take a non-const ref to it to pass it to something else anyway...)

> +
>  #else
>    // Check if the -stats option is set instead of checking
>    // !Stats.Stats.empty().  In release builds, Statistics operators
>    // do nothing, so stats are never Registered.
>    if (Enabled) {
>      // Get the stream to write to.
> -    raw_ostream &OutStream = *CreateInfoOutputFile();
> -    OutStream << "Statistics are disabled.  "
> -            << "Build with asserts or with -DLLVM_ENABLE_STATS\n";
> -    OutStream.flush();
> -    delete &OutStream;   // Close the file.
> +    std::unique_ptr<raw_ostream> OutStream = CreateInfoOutputFile();
> +    (*OutStream) << "Statistics are disabled.  "
> +                 << "Build with asserts or with -DLLVM_ENABLE_STATS\n";
>    }
>  #endif
>  }
>
> Modified: llvm/trunk/lib/Support/Timer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Timer.cpp?rev=255831&r1=255830&r2=255831&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/Timer.cpp (original)
> +++ llvm/trunk/lib/Support/Timer.cpp Wed Dec 16 16:28:34 2015
> @@ -12,6 +12,7 @@
>
>  //===----------------------------------------------------------------------===//
>
>  #include "llvm/Support/Timer.h"
> +#include "llvm/ADT/Statistic.h"
>  #include "llvm/ADT/StringMap.h"
>  #include "llvm/Support/CommandLine.h"
>  #include "llvm/Support/FileSystem.h"
> @@ -22,9 +23,6 @@
>  #include "llvm/Support/raw_ostream.h"
>  using namespace llvm;
>
> -// CreateInfoOutputFile - Return a file stream to print our output on.
> -namespace llvm { extern raw_ostream *CreateInfoOutputFile(); }
> -
>  // getLibSupportInfoOutputFilename - This ugly hack is brought to you
> courtesy
>  // of constructor/destructor ordering being unspecified by C++.
> Basically the
>  // problem is that a Statistic object gets destroyed, which ends up
> calling
> @@ -52,28 +50,27 @@ namespace {
>                     cl::Hidden,
> cl::location(getLibSupportInfoOutputFilename()));
>  }
>
> -// CreateInfoOutputFile - Return a file stream to print our output on.
> -raw_ostream *llvm::CreateInfoOutputFile() {
> +// Return a file stream to print our output on.
> +std::unique_ptr<raw_fd_ostream> llvm::CreateInfoOutputFile() {
>    const std::string &OutputFilename = getLibSupportInfoOutputFilename();
>    if (OutputFilename.empty())
> -    return new raw_fd_ostream(2, false); // stderr.
> +    return llvm::make_unique<raw_fd_ostream>(2, false); // stderr.
>    if (OutputFilename == "-")
> -    return new raw_fd_ostream(1, false); // stdout.
> -
> +    return llvm::make_unique<raw_fd_ostream>(1, false); // stdout.
> +
>    // Append mode is used because the info output file is opened and closed
>    // each time -stats or -time-passes wants to print output to it. To
>    // compensate for this, the test-suite Makefiles have code to delete the
>    // info output file before running commands which write to it.
>    std::error_code EC;
> -  raw_ostream *Result = new raw_fd_ostream(OutputFilename, EC,
> -                                           sys::fs::F_Append |
> sys::fs::F_Text);
> +  auto Result = llvm::make_unique<raw_fd_ostream>(
> +      OutputFilename, EC, sys::fs::F_Append | sys::fs::F_Text);
>    if (!EC)
>      return Result;
> -
> +
>    errs() << "Error opening info-output-file '"
>      << OutputFilename << " for appending!\n";
> -  delete Result;
> -  return new raw_fd_ostream(2, false); // stderr.
> +  return llvm::make_unique<raw_fd_ostream>(2, false); // stderr.
>  }
>
>
> @@ -292,10 +289,9 @@ void TimerGroup::removeTimer(Timer &T) {
>    // them were started.
>    if (FirstTimer || TimersToPrint.empty())
>      return;
> -
> -  raw_ostream *OutStream = CreateInfoOutputFile();
> +
> +  std::unique_ptr<raw_ostream> OutStream = CreateInfoOutputFile();
>    PrintQueuedTimers(*OutStream);
> -  delete OutStream;   // Close the file.
>  }
>
>  void TimerGroup::addTimer(Timer &T) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151217/d1cd0d85/attachment.html>