Changed BasicJsonDocument's copy-constructor to copy the capacity

CHANGELOG.md

@@ -13,6 +13,29 @@ HEAD
 * Fixed incorrect string comparison on some platforms (issue #1198)
 * Added move-constructor and move-assignment to `BasicJsonDocument`
 * Added `BasicJsonDocument::garbageCollect()` (issue #1195)
+* Changed copy-constructor of `BasicJsonDocument` to preserve the capacity of the source.
+
+> ### BREAKING CHANGES
+> 
+> #### Copy-constructor of `BasicJsonDocument`
+>
+> In previous versions, the copy constructor of `BasicJsonDocument` looked at the source's `memoryUsage()` to choose its capacity.
+> Now, the copy constructor of `BasicJsonDocument` uses the same capacity as the source.
+>
+> Example:
+>
+> ```c++
+> DynamicJsonDocument doc1(64);
+> doc1.set(String("example"));
+>
+> DynamicJsonDocument doc2 = doc1;
+> Serial.print(doc2.capacity());  // 8 with ArduinoJson 6.14
+                                  // 64 with ArduinoJson 6.15
+> ```
+>
+> I made this change to get consistent results between copy-constructor and move-constructor, and whether RVO applies or not.
+>
+> If you use the copy-constructor to optimize your documents, you can use `garbageCollect()` or `shrinkToFit()` instead.
 
 v6.14.1 (2020-01-27)
 -------

extras/tests/JsonDocument/BasicJsonDocument.cpp

@@ -20,7 +20,7 @@ class SpyingAllocator {
     return malloc(n);
   }
   void deallocate(void* p) {
-    _log << (p ? "F" : "f");
+    _log << "F";
     free(p);
   }
 
@@ -67,10 +67,12 @@ TEST_CASE("BasicJsonDocument") {
 
       REQUIRE(doc1.as<std::string>() == "The size of this string is 32!!");
       REQUIRE(doc2.as<std::string>() == "The size of this string is 32!!");
+      REQUIRE(doc2.capacity() == 4096);
     }
-    REQUIRE(log.str() == "A4096A32FF");
+    REQUIRE(log.str() == "A4096A4096FF");
   }
 
+#if ARDUINOJSON_HAS_RVALUE_REFERENCES
   SECTION("Move construct") {
     {
       BasicJsonDocument<SpyingAllocator> doc1(4096, log);
@@ -79,18 +81,13 @@ TEST_CASE("BasicJsonDocument") {
       BasicJsonDocument<SpyingAllocator> doc2(move(doc1));
 
       REQUIRE(doc2.as<std::string>() == "The size of this string is 32!!");
-#if ARDUINOJSON_HAS_RVALUE_REFERENCES
       REQUIRE(doc1.as<std::string>() == "null");
       REQUIRE(doc1.capacity() == 0);
       REQUIRE(doc2.capacity() == 4096);
-#endif
     }
-#if ARDUINOJSON_HAS_RVALUE_REFERENCES
+    REQUIRE(log.str() == "A4096F");
-    REQUIRE(log.str() == "A4096Ff");
-#else
-    REQUIRE(log.str() == "A4096A32FF");
-#endif
   }
+#endif
 
   SECTION("Copy assign") {
     {
@@ -102,10 +99,12 @@ TEST_CASE("BasicJsonDocument") {
 
       REQUIRE(doc1.as<std::string>() == "The size of this string is 32!!");
       REQUIRE(doc2.as<std::string>() == "The size of this string is 32!!");
+      REQUIRE(doc2.capacity() == 4096);
     }
-    REQUIRE(log.str() == "A4096A8FA32FF");
+    REQUIRE(log.str() == "A4096A8FA4096FF");
   }
 
+#if ARDUINOJSON_HAS_RVALUE_REFERENCES
   SECTION("Move assign") {
     {
       BasicJsonDocument<SpyingAllocator> doc1(4096, log);
@@ -115,18 +114,13 @@ TEST_CASE("BasicJsonDocument") {
       doc2 = move(doc1);
 
       REQUIRE(doc2.as<std::string>() == "The size of this string is 32!!");
-#if ARDUINOJSON_HAS_RVALUE_REFERENCES
       REQUIRE(doc1.as<std::string>() == "null");
       REQUIRE(doc1.capacity() == 0);
       REQUIRE(doc2.capacity() == 4096);
-#endif
     }
-#if ARDUINOJSON_HAS_RVALUE_REFERENCES
+    REQUIRE(log.str() == "A4096A8FF");
-    REQUIRE(log.str() == "A4096A8FFf");
-#else
-    REQUIRE(log.str() == "A4096A8FA32FF");
-#endif
   }
+#endif
 
   SECTION("garbageCollect()") {
     BasicJsonDocument<ControllableAllocator> doc(4096);

extras/tests/JsonDocument/DynamicJsonDocument.cpp

@@ -91,7 +91,7 @@ TEST_CASE("DynamicJsonDocument constructor") {
 
     REQUIRE_JSON(doc2, "{\"hello\":\"world\"}");
 
-    REQUIRE(doc2.capacity() == addPadding(doc1.memoryUsage()));
+    REQUIRE(doc2.capacity() == doc1.capacity());
   }
 
   SECTION("Construct from StaticJsonDocument") {
@@ -101,7 +101,7 @@ TEST_CASE("DynamicJsonDocument constructor") {
     DynamicJsonDocument doc2 = doc1;
 
     REQUIRE_JSON(doc2, "{\"hello\":\"world\"}");
-    REQUIRE(doc2.capacity() == addPadding(doc1.memoryUsage()));
+    REQUIRE(doc2.capacity() == doc1.capacity());
   }
 
   SECTION("Construct from JsonObject") {

src/ArduinoJson/Document/BasicJsonDocument.hpp

@@ -22,6 +22,7 @@ class AllocatorOwner {
   }
 
   void deallocate(void* ptr) {
+    if (ptr)
       _allocator.deallocate(ptr);
   }
 
@@ -43,27 +44,36 @@ class BasicJsonDocument : AllocatorOwner<TAllocator>, public JsonDocument {
   explicit BasicJsonDocument(size_t capa, TAllocator alloc = TAllocator())
       : AllocatorOwner<TAllocator>(alloc), JsonDocument(allocPool(capa)) {}
 
+  // Copy-constructor
   BasicJsonDocument(const BasicJsonDocument& src)
-      : AllocatorOwner<TAllocator>(src),
+      : AllocatorOwner<TAllocator>(src), JsonDocument() {
-        JsonDocument(allocPool(src.memoryUsage())) {
+    copyAssignFrom(src);
-    set(src);
   }
 
+  // Move-constructor
+#if ARDUINOJSON_HAS_RVALUE_REFERENCES
+  BasicJsonDocument(BasicJsonDocument&& src) : AllocatorOwner<TAllocator>(src) {
+    moveAssignFrom(src);
+  }
+#endif
+
+  BasicJsonDocument(const JsonDocument& src) {
+    copyAssignFrom(src);
+  }
+
+  // Construct from variant, array, or object
   template <typename T>
-  BasicJsonDocument(const T& src,
+  BasicJsonDocument(
-                    typename enable_if<IsVisitable<T>::value>::type* = 0)
+      const T& src,
+      typename enable_if<
+          is_same<T, VariantRef>::value || is_same<T, VariantConstRef>::value ||
+          is_same<T, ArrayRef>::value || is_same<T, ArrayConstRef>::value ||
+          is_same<T, ObjectRef>::value ||
+          is_same<T, ObjectConstRef>::value>::type* = 0)
       : JsonDocument(allocPool(src.memoryUsage())) {
     set(src);
   }
 
-#if ARDUINOJSON_HAS_RVALUE_REFERENCES
-  BasicJsonDocument(BasicJsonDocument&& src)
-      : AllocatorOwner<TAllocator>(src), JsonDocument(src) {
-    src._data.setNull();
-    src._pool = MemoryPool(0, 0);
-  }
-#endif
-
   // disambiguate
   BasicJsonDocument(VariantRef src)
       : JsonDocument(allocPool(src.memoryUsage())) {
@@ -75,8 +85,7 @@ class BasicJsonDocument : AllocatorOwner<TAllocator>, public JsonDocument {
   }
 
   BasicJsonDocument& operator=(const BasicJsonDocument& src) {
-    reallocPoolIfTooSmall(src.memoryUsage());
+    copyAssignFrom(src);
-    set(src);
     return *this;
   }
 
@@ -111,7 +120,7 @@ class BasicJsonDocument : AllocatorOwner<TAllocator>, public JsonDocument {
 
   bool garbageCollect() {
     // make a temporary clone and move assign
-    BasicJsonDocument<TAllocator> tmp(capacity(), allocator());
+    BasicJsonDocument<TAllocator> tmp(*this);
     if (!tmp.capacity())
       return false;
     tmp.set(*this);
@@ -138,6 +147,11 @@ class BasicJsonDocument : AllocatorOwner<TAllocator>, public JsonDocument {
     this->deallocate(memoryPool().buffer());
   }
 
+  void copyAssignFrom(const JsonDocument& src) {
+    reallocPoolIfTooSmall(src.capacity());
+    set(src);
+  }
+
   void moveAssignFrom(BasicJsonDocument& src) {
     freePool();
     _data = src._data;

src/ArduinoJson/Document/JsonDocument.hpp

@@ -293,6 +293,10 @@ class JsonDocument : public Visitable {
   }
 
  protected:
+  JsonDocument() : _pool(0, 0) {
+    _data.setNull();
+  }
+
   JsonDocument(MemoryPool pool) : _pool(pool) {
     _data.setNull();
   }