From 02f3b00ee6f8b0a6af51ea7ddd872129f0ca984a Mon Sep 17 00:00:00 2001 From: ecorm Date: Fri, 24 Oct 2014 02:51:17 -0300 Subject: [PATCH] Implemented C++11 move semantics for GenericDocument --- include/rapidjson/document.h | 49 +++++++- include/rapidjson/internal/stack.h | 52 ++++++++- test/unittest/documenttest.cpp | 180 +++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+), 5 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 4a3a1a2..f1ebdce 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -62,6 +62,10 @@ RAPIDJSON_DIAG_OFF(effc++) #include // std::iterator, std::random_access_iterator_tag #endif +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS +#include // std::move +#endif + namespace rapidjson { // Forward declaration. @@ -1623,9 +1627,46 @@ public: ownAllocator_ = allocator_ = new Allocator(); } - ~GenericDocument() { - delete ownAllocator_; +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS + //! Move constructor in C++11 + GenericDocument(GenericDocument&& rhs) RAPIDJSON_NOEXCEPT + : ValueType(std::move(rhs)), + allocator_(rhs.allocator_), + ownAllocator_(rhs.ownAllocator_), + stack_(std::move(rhs.stack_)), + parseResult_(rhs.parseResult_) + { + rhs.allocator_ = 0; + rhs.ownAllocator_ = 0; + rhs.parseResult_ = ParseResult(); } +#endif + + ~GenericDocument() { + Destroy(); + } + +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS + //! Move assignment in C++11 + GenericDocument& operator=(GenericDocument&& rhs) RAPIDJSON_NOEXCEPT + { + ValueType::operator=(std::move(static_cast(rhs))); + + // Calling the destructor here would prematurely call stack_'s destructor + Destroy(); + + allocator_ = rhs.allocator_; + ownAllocator_ = rhs.ownAllocator_; + stack_ = std::move(rhs.stack_); + parseResult_ = rhs.parseResult_; + + rhs.allocator_ = 0; + rhs.ownAllocator_ = 0; + rhs.parseResult_ = ParseResult(); + + return *this; + } +#endif //!@name Parse from stream //!@{ @@ -1821,6 +1862,10 @@ private: stack_.ShrinkToFit(); } + void Destroy() { + delete ownAllocator_; + } + static const size_t kDefaultStackCapacity = 1024; Allocator* allocator_; Allocator* ownAllocator_; diff --git a/include/rapidjson/internal/stack.h b/include/rapidjson/internal/stack.h index d4d3c92..ea722c6 100644 --- a/include/rapidjson/internal/stack.h +++ b/include/rapidjson/internal/stack.h @@ -41,10 +41,51 @@ public: ownAllocator = allocator_ = new Allocator(); } - ~Stack() { - Allocator::Free(stack_); - delete ownAllocator; // Only delete if it is owned by the stack +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS + Stack(Stack&& rhs) + : allocator_(rhs.allocator_), + ownAllocator(rhs.ownAllocator), + stack_(rhs.stack_), + stackTop_(rhs.stackTop_), + stackEnd_(rhs.stackEnd_), + initialCapacity_(rhs.initialCapacity_) + { + rhs.allocator_ = 0; + rhs.ownAllocator = 0; + rhs.stack_ = 0; + rhs.stackTop_ = 0; + rhs.stackEnd_ = 0; + rhs.initialCapacity_ = 0; } +#endif + + ~Stack() { + Destroy(); + } + +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS + Stack& operator=(Stack&& rhs) { + if (&rhs != this) + { + Destroy(); + + allocator_ = rhs.allocator_; + ownAllocator = rhs.ownAllocator; + stack_ = rhs.stack_; + stackTop_ = rhs.stackTop_; + stackEnd_ = rhs.stackEnd_; + initialCapacity_ = rhs.initialCapacity_; + + rhs.allocator_ = 0; + rhs.ownAllocator = 0; + rhs.stack_ = 0; + rhs.stackTop_ = 0; + rhs.stackEnd_ = 0; + rhs.initialCapacity_ = 0; + } + return *this; + } +#endif void Clear() { stackTop_ = stack_; } @@ -119,6 +160,11 @@ private: stackEnd_ = stack_ + newCapacity; } + void Destroy() { + Allocator::Free(stack_); + delete ownAllocator; // Only delete if it is owned by the stack + } + // Prohibit copy constructor & assignment operator. Stack(const Stack&); Stack& operator=(const Stack&); diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 5809361..7295aef 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -227,6 +227,186 @@ TEST(Document, UTF16_Document) { EXPECT_EQ(0, wcscmp(L"Wed Oct 30 17:13:20 +0000 2012", s.GetString())); } +#if RAPIDJSON_HAS_CXX11_RVALUE_REFS + +TEST(Document, MoveConstructor) { + typedef GenericDocument, CrtAllocator> Document; + Document::AllocatorType allocator; + + Document a(&allocator); + a.Parse("[\"one\", \"two\", \"three\"]"); + EXPECT_FALSE(a.HasParseError()); + EXPECT_TRUE(a.IsArray()); + EXPECT_EQ(3u, a.Size()); + EXPECT_EQ(&a.GetAllocator(), &allocator); + + // Document b(a); // should not compile + Document b(std::move(a)); + EXPECT_TRUE(a.IsNull()); + EXPECT_TRUE(b.IsArray()); + EXPECT_EQ(3u, b.Size()); + EXPECT_EQ(&a.GetAllocator(), (void*)0); + EXPECT_EQ(&b.GetAllocator(), &allocator); + + b.Parse("{\"Foo\": \"Bar\", \"Baz\": 42}"); + EXPECT_FALSE(b.HasParseError()); + EXPECT_TRUE(b.IsObject()); + EXPECT_EQ(2u, b.MemberCount()); + + // Document c = a; // should not compile + Document c = std::move(b); + EXPECT_TRUE(b.IsNull()); + EXPECT_TRUE(c.IsObject()); + EXPECT_EQ(2u, c.MemberCount()); + EXPECT_EQ(&b.GetAllocator(), (void*)0); + EXPECT_EQ(&c.GetAllocator(), &allocator); +} + +TEST(Document, MoveConstructorParseError) { + typedef GenericDocument, CrtAllocator> Document; + + ParseResult noError; + Document a; + a.Parse("{ 4 = 4]"); + ParseResult error(a.GetParseError(), a.GetErrorOffset()); + EXPECT_TRUE(a.HasParseError()); + EXPECT_NE(error.Code(), noError.Code()); + EXPECT_NE(error.Offset(), noError.Offset()); + + Document b(std::move(a)); + EXPECT_FALSE(a.HasParseError()); + EXPECT_TRUE(b.HasParseError()); + EXPECT_EQ(a.GetParseError(), noError.Code()); + EXPECT_EQ(b.GetParseError(), error.Code()); + EXPECT_EQ(a.GetErrorOffset(), noError.Offset()); + EXPECT_EQ(b.GetErrorOffset(), error.Offset()); + + Document c(std::move(b)); + EXPECT_FALSE(b.HasParseError()); + EXPECT_TRUE(c.HasParseError()); + EXPECT_EQ(b.GetParseError(), noError.Code()); + EXPECT_EQ(c.GetParseError(), error.Code()); + EXPECT_EQ(b.GetErrorOffset(), noError.Offset()); + EXPECT_EQ(c.GetErrorOffset(), error.Offset()); +} + +TEST(Document, MoveConstructorStack) { + typedef UTF8<> Encoding; + typedef GenericDocument Document; + + Document a; + size_t defaultCapacity = a.GetStackCapacity(); + + // Trick Document into getting GetStackCapacity() to return non-zero + typedef GenericReader Reader; + Reader reader(&a.GetAllocator()); + GenericStringStream is("[\"one\", \"two\", \"three\"]"); + reader.Parse(is, a); + size_t capacity = a.GetStackCapacity(); + EXPECT_GT(capacity, 0); + + Document b(std::move(a)); + EXPECT_EQ(a.GetStackCapacity(), defaultCapacity); + EXPECT_EQ(b.GetStackCapacity(), capacity); + + Document c = std::move(b); + EXPECT_EQ(b.GetStackCapacity(), defaultCapacity); + EXPECT_EQ(c.GetStackCapacity(), capacity); +} + +TEST(Document, MoveAssignment) { + typedef GenericDocument, CrtAllocator> Document; + Document::AllocatorType allocator; + + Document a(&allocator); + a.Parse("[\"one\", \"two\", \"three\"]"); + EXPECT_FALSE(a.HasParseError()); + EXPECT_TRUE(a.IsArray()); + EXPECT_EQ(3u, a.Size()); + EXPECT_EQ(&a.GetAllocator(), &allocator); + + // Document b; b = a; // should not compile + Document b; + b = std::move(a); + EXPECT_TRUE(a.IsNull()); + EXPECT_TRUE(b.IsArray()); + EXPECT_EQ(3u, b.Size()); + EXPECT_EQ(&a.GetAllocator(), (void*)0); + EXPECT_EQ(&b.GetAllocator(), &allocator); + + b.Parse("{\"Foo\": \"Bar\", \"Baz\": 42}"); + EXPECT_FALSE(b.HasParseError()); + EXPECT_TRUE(b.IsObject()); + EXPECT_EQ(2u, b.MemberCount()); + + // Document c; c = a; // should not compile + Document c; + c = std::move(b); + EXPECT_TRUE(b.IsNull()); + EXPECT_TRUE(c.IsObject()); + EXPECT_EQ(2u, c.MemberCount()); + EXPECT_EQ(&b.GetAllocator(), (void*)0); + EXPECT_EQ(&c.GetAllocator(), &allocator); +} + +TEST(Document, MoveAssignmentParseError) { + typedef GenericDocument, CrtAllocator> Document; + + ParseResult noError; + Document a; + a.Parse("{ 4 = 4]"); + ParseResult error(a.GetParseError(), a.GetErrorOffset()); + EXPECT_TRUE(a.HasParseError()); + EXPECT_NE(error.Code(), noError.Code()); + EXPECT_NE(error.Offset(), noError.Offset()); + + Document b; + b = std::move(a); + EXPECT_FALSE(a.HasParseError()); + EXPECT_TRUE(b.HasParseError()); + EXPECT_EQ(a.GetParseError(), noError.Code()); + EXPECT_EQ(b.GetParseError(), error.Code()); + EXPECT_EQ(a.GetErrorOffset(), noError.Offset()); + EXPECT_EQ(b.GetErrorOffset(), error.Offset()); + + Document c; + c = std::move(b); + EXPECT_FALSE(b.HasParseError()); + EXPECT_TRUE(c.HasParseError()); + EXPECT_EQ(b.GetParseError(), noError.Code()); + EXPECT_EQ(c.GetParseError(), error.Code()); + EXPECT_EQ(b.GetErrorOffset(), noError.Offset()); + EXPECT_EQ(c.GetErrorOffset(), error.Offset()); +} + +TEST(Document, MoveAssignmentStack) { + typedef UTF8<> Encoding; + typedef GenericDocument Document; + + Document a; + size_t defaultCapacity = a.GetStackCapacity(); + + // Trick Document into getting GetStackCapacity() to return non-zero + typedef GenericReader Reader; + Reader reader(&a.GetAllocator()); + GenericStringStream is("[\"one\", \"two\", \"three\"]"); + reader.Parse(is, a); + size_t capacity = a.GetStackCapacity(); + EXPECT_GT(capacity, 0); + + Document b; + b = std::move(a); + EXPECT_EQ(a.GetStackCapacity(), defaultCapacity); + EXPECT_EQ(b.GetStackCapacity(), capacity); + + Document c; + c = std::move(b); + EXPECT_EQ(b.GetStackCapacity(), defaultCapacity); + EXPECT_EQ(c.GetStackCapacity(), capacity); +} + +#endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS + // Issue 22: Memory corruption via operator= // Fixed by making unimplemented assignment operator private. //TEST(Document, Assignment) {