[PATCH] D36942: Defaultify copy and move constructors for StreamRefs

[Alex Telishev via Phabricator via llvm-commits]

alex.telishev created this revision.
Herald added a subscriber: hiraditya.

Only copy constructor was present for `BinaryStreamRefBase`  and `BinaryStreamRef` resulting in move constructor being deleted.  `BinaryStreamRefBase` contains `std::shared_ptr` that is quite expensive to copy and cheap to move. `BinaryStreamRef` is moved quite often (as it's used as return type of many functions).


https://reviews.llvm.org/D36942

Files:
  llvm/include/llvm/Support/BinaryStreamRef.h
  llvm/lib/Support/BinaryStreamRef.cpp


Index: llvm/lib/Support/BinaryStreamRef.cpp
===================================================================
--- llvm/lib/Support/BinaryStreamRef.cpp
+++ llvm/lib/Support/BinaryStreamRef.cpp
@@ -77,9 +77,6 @@
     : BinaryStreamRef(makeArrayRef(Data.bytes_begin(), Data.bytes_end()),
                       Endian) {}
 
-BinaryStreamRef::BinaryStreamRef(const BinaryStreamRef &Other)
-    : BinaryStreamRefBase(Other) {}
-
 Error BinaryStreamRef::readBytes(uint32_t Offset, uint32_t Size,
                                  ArrayRef<uint8_t> &Buffer) const {
   if (auto EC = checkOffset(Offset, Size))
@@ -117,9 +114,6 @@
     : BinaryStreamRefBase(std::make_shared<MutableArrayRefImpl>(Data, Endian),
                           0, Data.size()) {}
 
-WritableBinaryStreamRef::WritableBinaryStreamRef(
-    const WritableBinaryStreamRef &Other)
-    : BinaryStreamRefBase(Other) {}
 
 Error WritableBinaryStreamRef::writeBytes(uint32_t Offset,
                                           ArrayRef<uint8_t> Data) const {
Index: llvm/include/llvm/Support/BinaryStreamRef.h
===================================================================
--- llvm/include/llvm/Support/BinaryStreamRef.h
+++ llvm/include/llvm/Support/BinaryStreamRef.h
@@ -31,12 +31,11 @@
   BinaryStreamRefBase(StreamType &BorrowedImpl, uint32_t Offset,
                       uint32_t Length)
       : BorrowedImpl(&BorrowedImpl), ViewOffset(Offset), Length(Length) {}
-  BinaryStreamRefBase(const BinaryStreamRefBase &Other) {
-    SharedImpl = Other.SharedImpl;
-    BorrowedImpl = Other.BorrowedImpl;
-    ViewOffset = Other.ViewOffset;
-    Length = Other.Length;
-  }
+  BinaryStreamRefBase(const BinaryStreamRefBase &Other) = default;
+  BinaryStreamRefBase &operator=(const BinaryStreamRefBase &Other) = default;
+
+  BinaryStreamRefBase &operator=(BinaryStreamRefBase &&Other) = default;
+  BinaryStreamRefBase(BinaryStreamRefBase &&Other) = default;
 
 public:
   llvm::support::endianness getEndian() const {
@@ -142,7 +141,10 @@
                            llvm::support::endianness Endian);
   explicit BinaryStreamRef(StringRef Data, llvm::support::endianness Endian);
 
-  BinaryStreamRef(const BinaryStreamRef &Other);
+  BinaryStreamRef(const BinaryStreamRef &Other) = default;
+  BinaryStreamRef &operator=(const BinaryStreamRef &Other) = default;
+  BinaryStreamRef(BinaryStreamRef &&Other) = default;
+  BinaryStreamRef &operator=(BinaryStreamRef &&Other) = default;
 
   // Use BinaryStreamRef.slice() instead.
   BinaryStreamRef(BinaryStreamRef &S, uint32_t Offset,
@@ -203,7 +205,11 @@
                           uint32_t Length);
   explicit WritableBinaryStreamRef(MutableArrayRef<uint8_t> Data,
                                    llvm::support::endianness Endian);
-  WritableBinaryStreamRef(const WritableBinaryStreamRef &Other);
+  WritableBinaryStreamRef(const WritableBinaryStreamRef &Other) = default;
+  WritableBinaryStreamRef &operator= (const WritableBinaryStreamRef &Other) = default;
+
+  WritableBinaryStreamRef(WritableBinaryStreamRef &&Other) = default;
+  WritableBinaryStreamRef &operator= (WritableBinaryStreamRef &&Other) = default;
 
   // Use WritableBinaryStreamRef.slice() instead.
   WritableBinaryStreamRef(WritableBinaryStreamRef &S, uint32_t Offset,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36942.111905.patch
Type: text/x-patch
Size: 3271 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170820/983d68ef/attachment.bin>