From 7c1f20825374390e2a1005e0bc488a3b99c873d0 Mon Sep 17 00:00:00 2001 From: bluehero Date: Sat, 5 Aug 2017 16:53:45 +0800 Subject: [PATCH 1/6] modify --- include/rapidjson/document.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 3169bd4..f5c02d6 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -2168,6 +2168,10 @@ public: } #endif + // Allow assignment from ValueType. + // Refer to Effective C++ 3rd Edition/Item 33: Avoid hiding inherited names. + using ValueType::operator=; + //! Exchange the contents of this document with those of another. /*! \param rhs Another document. @@ -2183,6 +2187,10 @@ public: return *this; } + // Allow Swap from ValueType. + // Refer to Effective C++ 3rd Edition/Item 33: Avoid hiding inherited names. + using ValueType::Swap; + //! free-standing swap function helper /*! Helper function to enable support for common swap implementation pattern based on \c std::swap: From 9eb7bf895c124fcf76877b173f6930a40e71e0a8 Mon Sep 17 00:00:00 2001 From: bluehero Date: Sat, 5 Aug 2017 18:12:44 +0800 Subject: [PATCH 2/6] add unittest --- test/unittest/documenttest.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 0ca5801..55d828a 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -290,6 +290,14 @@ TEST(Document, ParseStream_AutoUTFInputStream) { EXPECT_EQ(0, memcmp(bos.GetString(), bos2.GetString(), bos2.GetSize())); } +TEST(Document, Assignment) { + Value x(1234); + Document d; + d = x; + EXPECT_TRUE(x.IsNull()); // move semantic + EXPECT_EQ(1234, d.GetInt()); +} + TEST(Document, Swap) { Document d1; Document::AllocatorType& a = d1.GetAllocator(); @@ -300,7 +308,14 @@ TEST(Document, Swap) { o.SetObject().AddMember("a", 1, a); // Swap between Document and Value - // d1.Swap(o); // doesn't compile + d1.Swap(o); + EXPECT_TRUE(d1.IsObject()); + EXPECT_TRUE(o.IsArray()); + + d1.Swap(o); + EXPECT_TRUE(d1.IsArray()); + EXPECT_TRUE(o.IsObject()); + o.Swap(d1); EXPECT_TRUE(d1.IsObject()); EXPECT_TRUE(o.IsArray()); From 8ba1f84f47a3c5761be86884f77421a73c9a38fe Mon Sep 17 00:00:00 2001 From: bluehero Date: Sat, 5 Aug 2017 20:39:31 +0800 Subject: [PATCH 3/6] modify unittest --- test/unittest/documenttest.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 55d828a..9ff8096 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -290,14 +290,6 @@ TEST(Document, ParseStream_AutoUTFInputStream) { EXPECT_EQ(0, memcmp(bos.GetString(), bos2.GetString(), bos2.GetSize())); } -TEST(Document, Assignment) { - Value x(1234); - Document d; - d = x; - EXPECT_TRUE(x.IsNull()); // move semantic - EXPECT_EQ(1234, d.GetInt()); -} - TEST(Document, Swap) { Document d1; Document::AllocatorType& a = d1.GetAllocator(); @@ -667,13 +659,20 @@ TYPED_TEST(DocumentMove, MoveAssignmentStack) { #endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS -// Issue 22: Memory corruption via operator= +// Issue 22: Memory corruption via operator= from Document // Fixed by making unimplemented assignment operator private. -//TEST(Document, Assignment) { +// Prohibit assignment from Document, But allow assignment from Value +TEST(Document, Assignment) { // Document d1; // Document d2; // d1 = d2; -//} + + Value x(1234); + Document d; + d = x; + EXPECT_TRUE(x.IsNull()); // move semantic + EXPECT_EQ(1234, d.GetInt()); +} #ifdef __clang__ RAPIDJSON_DIAG_POP From 5fb06596a93f62e98fb9900dac2f1c97e5981549 Mon Sep 17 00:00:00 2001 From: bluehero Date: Mon, 7 Aug 2017 11:44:27 +0800 Subject: [PATCH 4/6] modify --- include/rapidjson/document.h | 4 +--- test/unittest/documenttest.cpp | 12 ++++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index f5c02d6..869667a 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -2168,8 +2168,7 @@ public: } #endif - // Allow assignment from ValueType. - // Refer to Effective C++ 3rd Edition/Item 33: Avoid hiding inherited names. + // Allow assignment like a ValueType. using ValueType::operator=; //! Exchange the contents of this document with those of another. @@ -2188,7 +2187,6 @@ public: } // Allow Swap from ValueType. - // Refer to Effective C++ 3rd Edition/Item 33: Avoid hiding inherited names. using ValueType::Swap; //! free-standing swap function helper diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 9ff8096..0d08b25 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -661,17 +661,25 @@ TYPED_TEST(DocumentMove, MoveAssignmentStack) { // Issue 22: Memory corruption via operator= from Document // Fixed by making unimplemented assignment operator private. -// Prohibit assignment from Document, But allow assignment from Value +// Prohibit assignment from Document. +// But allow assignment from ValueType/int/double/..., like a ValueType TEST(Document, Assignment) { // Document d1; // Document d2; // d1 = d2; - Value x(1234); Document d; + + Value x(1234); d = x; EXPECT_TRUE(x.IsNull()); // move semantic EXPECT_EQ(1234, d.GetInt()); + + d = 1; + EXPECT_EQ(1, d.GetInt()); + + d = 12.34; + EXPECT_NEAR(12.34, d.GetDouble(), 0.0); } #ifdef __clang__ From c831675026cc2c0a7b3581d8b0e0fe4eedd8d78f Mon Sep 17 00:00:00 2001 From: bluehero Date: Mon, 7 Aug 2017 11:58:37 +0800 Subject: [PATCH 5/6] modify --- include/rapidjson/document.h | 6 ++---- test/unittest/documenttest.cpp | 21 +++------------------ 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 869667a..f55b7d3 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -2168,9 +2168,6 @@ public: } #endif - // Allow assignment like a ValueType. - using ValueType::operator=; - //! Exchange the contents of this document with those of another. /*! \param rhs Another document. @@ -2186,7 +2183,8 @@ public: return *this; } - // Allow Swap from ValueType. + // Allow Swap with ValueType. + // Refer to Effective C++/Item 33: Avoid hiding inherited names. using ValueType::Swap; //! free-standing swap function helper diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 0d08b25..5429802 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -659,28 +659,13 @@ TYPED_TEST(DocumentMove, MoveAssignmentStack) { #endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS -// Issue 22: Memory corruption via operator= from Document +// Issue 22: Memory corruption via operator= // Fixed by making unimplemented assignment operator private. -// Prohibit assignment from Document. -// But allow assignment from ValueType/int/double/..., like a ValueType -TEST(Document, Assignment) { +//TEST(Document, Assignment) { // Document d1; // Document d2; // d1 = d2; - - Document d; - - Value x(1234); - d = x; - EXPECT_TRUE(x.IsNull()); // move semantic - EXPECT_EQ(1234, d.GetInt()); - - d = 1; - EXPECT_EQ(1, d.GetInt()); - - d = 12.34; - EXPECT_NEAR(12.34, d.GetDouble(), 0.0); -} +//} #ifdef __clang__ RAPIDJSON_DIAG_POP From f9004b90c555f9374d3f6e4d462e4abbce3b00a8 Mon Sep 17 00:00:00 2001 From: bluehero Date: Mon, 7 Aug 2017 13:09:22 +0800 Subject: [PATCH 6/6] modify --- include/rapidjson/document.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index f55b7d3..3133a2f 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -2184,7 +2184,7 @@ public: } // Allow Swap with ValueType. - // Refer to Effective C++/Item 33: Avoid hiding inherited names. + // Refer to Effective C++ 3rd Edition/Item 33: Avoid hiding inherited names. using ValueType::Swap; //! free-standing swap function helper