r215853 - Convert a few ownership comments with std::unique_ptr.

[David Blaikie]

On Sun, Aug 17, 2014 at 3:12 PM, Rafael Espindola
<rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Sun Aug 17 17:12:58 2014
> New Revision: 215853
>
> URL: http://llvm.org/viewvc/llvm-project?rev=215853&view=rev
> Log:
> Convert a few ownership comments with std::unique_ptr.
>
> Modified:
>     cfe/trunk/include/clang/Basic/VirtualFileSystem.h
>     cfe/trunk/lib/Basic/VirtualFileSystem.cpp
>     cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>     cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>
> Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=215853&r1=215852&r2=215853&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
> +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Sun Aug 17 17:12:58 2014
> @@ -250,10 +250,8 @@ llvm::sys::fs::UniqueID getNextVirtualUn
>
>  /// \brief Gets a \p FileSystem for a virtual file system described in YAML
>  /// format.
> -///
> -/// Takes ownership of \p Buffer.
>  IntrusiveRefCntPtr<FileSystem>
> -getVFSFromYAML(llvm::MemoryBuffer *Buffer,
> +getVFSFromYAML(std::unique_ptr<llvm::MemoryBuffer> Buffer,
>                 llvm::SourceMgr::DiagHandlerTy DiagHandler,
>                 void *DiagContext = nullptr,
>                 IntrusiveRefCntPtr<FileSystem> ExternalFS = getRealFileSystem());
>
> Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=215853&r1=215852&r2=215853&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Sun Aug 17 17:12:58 2014
> @@ -514,9 +514,7 @@ public:
>
>    /// \brief Parses \p Buffer, which is expected to be in YAML format and
>    /// returns a virtual file system representing its contents.
> -  ///
> -  /// Takes ownership of \p Buffer.
> -  static VFSFromYAML *create(MemoryBuffer *Buffer,
> +  static VFSFromYAML *create(std::unique_ptr<MemoryBuffer> Buffer,
>                               SourceMgr::DiagHandlerTy DiagHandler,
>                               void *DiagContext,
>                               IntrusiveRefCntPtr<FileSystem> ExternalFS);
> @@ -865,13 +863,13 @@ DirectoryEntry::~DirectoryEntry() { llvm
>
>  VFSFromYAML::~VFSFromYAML() { llvm::DeleteContainerPointers(Roots); }
>
> -VFSFromYAML *VFSFromYAML::create(MemoryBuffer *Buffer,
> +VFSFromYAML *VFSFromYAML::create(std::unique_ptr<MemoryBuffer> Buffer,
>                                   SourceMgr::DiagHandlerTy DiagHandler,
>                                   void *DiagContext,
>                                   IntrusiveRefCntPtr<FileSystem> ExternalFS) {
>
>    SourceMgr SM;
> -  yaml::Stream Stream(Buffer, SM);
> +  yaml::Stream Stream(Buffer.release(), SM);
>
>    SM.setDiagHandler(DiagHandler, DiagContext);
>    yaml::document_iterator DI = Stream.begin();
> @@ -993,10 +991,11 @@ VFSFromYAML::openFileForRead(const Twine
>  }
>
>  IntrusiveRefCntPtr<FileSystem>
> -vfs::getVFSFromYAML(MemoryBuffer *Buffer, SourceMgr::DiagHandlerTy DiagHandler,
> -                    void *DiagContext,
> +vfs::getVFSFromYAML(std::unique_ptr<MemoryBuffer> Buffer,
> +                    SourceMgr::DiagHandlerTy DiagHandler, void *DiagContext,
>                      IntrusiveRefCntPtr<FileSystem> ExternalFS) {
> -  return VFSFromYAML::create(Buffer, DiagHandler, DiagContext, ExternalFS);
> +  return VFSFromYAML::create(std::move(Buffer), DiagHandler, DiagContext,
> +                             ExternalFS);
>  }
>
>  UniqueID vfs::getNextVirtualUniqueID() {
>
> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=215853&r1=215852&r2=215853&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Sun Aug 17 17:12:58 2014
> @@ -2034,7 +2034,7 @@ createVFSFromCompilerInvocation(const Co
>      }
>
>      IntrusiveRefCntPtr<vfs::FileSystem> FS =
> -        vfs::getVFSFromYAML(Buffer->release(), /*DiagHandler*/ nullptr);
> +        vfs::getVFSFromYAML(std::move(Buffer.get()), /*DiagHandler*/ nullptr);
>      if (!FS.get()) {
>        Diags.Report(diag::err_invalid_vfs_overlay) << File;
>        return IntrusiveRefCntPtr<vfs::FileSystem>();
>
> Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=215853&r1=215852&r2=215853&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
> +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Sun Aug 17 17:12:58 2014
> @@ -540,7 +540,8 @@ public:
>    getFromYAMLRawString(StringRef Content,
>                         IntrusiveRefCntPtr<vfs::FileSystem> ExternalFS) {
>      MemoryBuffer *Buffer = MemoryBuffer::getMemBuffer(Content);
> -    return getVFSFromYAML(Buffer, CountingDiagHandler, this, ExternalFS);
> +    return getVFSFromYAML(std::unique_ptr<MemoryBuffer>(Buffer),

Directly constructing temporary smart pointers is an eye-brow raising
thing (though not applicable to LLVM - it's not exception safe). I'd
probably be inclined to take ownership on the line before (with the
result of getMemBuffer) into a named unique_ptr, then move into the
call got getVFSFromYAML instead.

(& perhaps one day we'll have MemoryBuffer's factories return by
unique_ptr... - or we'll get rid of its polymorphism/ownership stuff
and just have value-passed MemoryBufferRefs... one day)

> +                          CountingDiagHandler, this, ExternalFS);
>    }
>
>    IntrusiveRefCntPtr<vfs::FileSystem> getFromYAMLString(
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits