diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c2e3cc..3a9ed809 100644 --- a/CHANGELOG.md +++ b/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) ------- diff --git a/extras/tests/JsonDocument/BasicJsonDocument.cpp b/extras/tests/JsonDocument/BasicJsonDocument.cpp index f0672f32..6f068f4d 100644 --- a/extras/tests/JsonDocument/BasicJsonDocument.cpp +++ b/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() == "The size of this string is 32!!"); REQUIRE(doc2.as() == "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 doc1(4096, log); @@ -79,18 +81,13 @@ TEST_CASE("BasicJsonDocument") { BasicJsonDocument doc2(move(doc1)); REQUIRE(doc2.as() == "The size of this string is 32!!"); -#if ARDUINOJSON_HAS_RVALUE_REFERENCES REQUIRE(doc1.as() == "null"); REQUIRE(doc1.capacity() == 0); REQUIRE(doc2.capacity() == 4096); -#endif } -#if ARDUINOJSON_HAS_RVALUE_REFERENCES - REQUIRE(log.str() == "A4096Ff"); -#else - REQUIRE(log.str() == "A4096A32FF"); -#endif + REQUIRE(log.str() == "A4096F"); } +#endif SECTION("Copy assign") { { @@ -102,10 +99,12 @@ TEST_CASE("BasicJsonDocument") { REQUIRE(doc1.as() == "The size of this string is 32!!"); REQUIRE(doc2.as() == "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 doc1(4096, log); @@ -115,18 +114,13 @@ TEST_CASE("BasicJsonDocument") { doc2 = move(doc1); REQUIRE(doc2.as() == "The size of this string is 32!!"); -#if ARDUINOJSON_HAS_RVALUE_REFERENCES REQUIRE(doc1.as() == "null"); REQUIRE(doc1.capacity() == 0); REQUIRE(doc2.capacity() == 4096); -#endif } -#if ARDUINOJSON_HAS_RVALUE_REFERENCES - REQUIRE(log.str() == "A4096A8FFf"); -#else - REQUIRE(log.str() == "A4096A8FA32FF"); -#endif + REQUIRE(log.str() == "A4096A8FF"); } +#endif SECTION("garbageCollect()") { BasicJsonDocument doc(4096); diff --git a/extras/tests/JsonDocument/DynamicJsonDocument.cpp b/extras/tests/JsonDocument/DynamicJsonDocument.cpp index c6fd6106..6c71eb8e 100644 --- a/extras/tests/JsonDocument/DynamicJsonDocument.cpp +++ b/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") { diff --git a/src/ArduinoJson/Document/BasicJsonDocument.hpp b/src/ArduinoJson/Document/BasicJsonDocument.hpp index a9e9f88d..a539659f 100644 --- a/src/ArduinoJson/Document/BasicJsonDocument.hpp +++ b/src/ArduinoJson/Document/BasicJsonDocument.hpp @@ -22,7 +22,8 @@ class AllocatorOwner { } void deallocate(void* ptr) { - _allocator.deallocate(ptr); + if (ptr) + _allocator.deallocate(ptr); } void* reallocate(void* ptr, size_t new_size) { @@ -43,27 +44,36 @@ class BasicJsonDocument : AllocatorOwner, public JsonDocument { explicit BasicJsonDocument(size_t capa, TAllocator alloc = TAllocator()) : AllocatorOwner(alloc), JsonDocument(allocPool(capa)) {} + // Copy-constructor BasicJsonDocument(const BasicJsonDocument& src) - : AllocatorOwner(src), - JsonDocument(allocPool(src.memoryUsage())) { - set(src); + : AllocatorOwner(src), JsonDocument() { + copyAssignFrom(src); } + // Move-constructor +#if ARDUINOJSON_HAS_RVALUE_REFERENCES + BasicJsonDocument(BasicJsonDocument&& src) : AllocatorOwner(src) { + moveAssignFrom(src); + } +#endif + + BasicJsonDocument(const JsonDocument& src) { + copyAssignFrom(src); + } + + // Construct from variant, array, or object template - BasicJsonDocument(const T& src, - typename enable_if::value>::type* = 0) + BasicJsonDocument( + const T& src, + typename enable_if< + is_same::value || is_same::value || + is_same::value || is_same::value || + is_same::value || + is_same::value>::type* = 0) : JsonDocument(allocPool(src.memoryUsage())) { set(src); } -#if ARDUINOJSON_HAS_RVALUE_REFERENCES - BasicJsonDocument(BasicJsonDocument&& src) - : AllocatorOwner(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, public JsonDocument { } BasicJsonDocument& operator=(const BasicJsonDocument& src) { - reallocPoolIfTooSmall(src.memoryUsage()); - set(src); + copyAssignFrom(src); return *this; } @@ -111,7 +120,7 @@ class BasicJsonDocument : AllocatorOwner, public JsonDocument { bool garbageCollect() { // make a temporary clone and move assign - BasicJsonDocument tmp(capacity(), allocator()); + BasicJsonDocument tmp(*this); if (!tmp.capacity()) return false; tmp.set(*this); @@ -138,6 +147,11 @@ class BasicJsonDocument : AllocatorOwner, public JsonDocument { this->deallocate(memoryPool().buffer()); } + void copyAssignFrom(const JsonDocument& src) { + reallocPoolIfTooSmall(src.capacity()); + set(src); + } + void moveAssignFrom(BasicJsonDocument& src) { freePool(); _data = src._data; diff --git a/src/ArduinoJson/Document/JsonDocument.hpp b/src/ArduinoJson/Document/JsonDocument.hpp index 915d8a8e..242d801a 100644 --- a/src/ArduinoJson/Document/JsonDocument.hpp +++ b/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(); }