From e294ce6f7a32c5823460eb0160254c21140454e8 Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sat, 30 Aug 2014 18:34:49 +0200 Subject: [PATCH 1/7] GenericValue: accept different allocators for read-only access Several GenericValue functions take a const-reference to another GenericValue only. These functions can safely accept values with a different allocator type. The following functions are therefore extended by a SourceAllocator template parameter in this commit: - operator==/!=/[] - HasMember, FindMember, RemoveMember - StringEqual --- include/rapidjson/document.h | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 92c7316..8adfd7d 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -444,7 +444,7 @@ public: \see CopyFrom() */ template< typename SourceAllocator > - GenericValue(const GenericValue& rhs, Allocator & allocator); + GenericValue(const GenericValue& rhs, Allocator & allocator); //! Constructor for boolean value. /*! \param b Boolean value @@ -603,10 +603,10 @@ public: \param allocator Allocator to use for copying */ template - GenericValue& CopyFrom(const GenericValue& rhs, Allocator& allocator) { + GenericValue& CopyFrom(const GenericValue& rhs, Allocator& allocator) { RAPIDJSON_ASSERT((void*)this != (void const*)&rhs); this->~GenericValue(); - new (this) GenericValue(rhs,allocator); + new (this) GenericValue(rhs, allocator); return *this; } @@ -635,7 +635,9 @@ public: \note If an object contains duplicated named member, comparing equality with any object is always \c false. \note Linear time complexity (number of all values in the subtree and total lengths of all strings). */ - bool operator==(const GenericValue& rhs) const { + template + bool operator==(const GenericValue& rhs) const { + typedef GenericValue RhsType; if (GetType() != rhs.GetType()) return false; @@ -644,7 +646,7 @@ public: if (data_.o.size != rhs.data_.o.size) return false; for (ConstMemberIterator lhsMemberItr = MemberBegin(); lhsMemberItr != MemberEnd(); ++lhsMemberItr) { - ConstMemberIterator rhsMemberItr = rhs.FindMember(lhsMemberItr->name); + typename RhsType::ConstMemberIterator rhsMemberItr = rhs.FindMember(lhsMemberItr->name); if (rhsMemberItr == rhs.MemberEnd() || lhsMemberItr->value != rhsMemberItr->value) return false; } @@ -690,7 +692,8 @@ public: //! Not-equal-to operator /*! \return !(*this == rhs) */ - bool operator!=(const GenericValue& rhs) const { return !(*this == rhs); } + template + bool operator!=(const GenericValue& rhs) const { return !(*this == rhs); } //! Not-equal-to operator with arbitrary types /*! \return !(*this == rhs) @@ -774,7 +777,8 @@ public: // This version is faster because it does not need a StrLen(). // It can also handle string with null character. - GenericValue& operator[](const GenericValue& name) { + template + GenericValue& operator[](const GenericValue& name) { MemberIterator member = FindMember(name); if (member != MemberEnd()) return member->value; @@ -784,7 +788,8 @@ public: return NullValue; } } - const GenericValue& operator[](const GenericValue& name) const { return const_cast(*this)[name]; } + template + const GenericValue& operator[](const GenericValue& name) const { return const_cast(*this)[name]; } //! Const member iterator /*! \pre IsObject() == true */ @@ -818,7 +823,8 @@ public: \note It is better to use FindMember() directly if you need the obtain the value as well. \note Linear time complexity. */ - bool HasMember(const GenericValue& name) const { return FindMember(name) != MemberEnd(); } + template + bool HasMember(const GenericValue& name) const { return FindMember(name) != MemberEnd(); } //! Find member by name. /*! @@ -852,7 +858,8 @@ public: \c std::map, this has been changed to MemberEnd() now. \note Linear time complexity. */ - MemberIterator FindMember(const GenericValue& name) { + template + MemberIterator FindMember(const GenericValue& name) { RAPIDJSON_ASSERT(IsObject()); RAPIDJSON_ASSERT(name.IsString()); MemberIterator member = MemberBegin(); @@ -861,7 +868,7 @@ public: break; return member; } - ConstMemberIterator FindMember(const GenericValue& name) const { return const_cast(*this).FindMember(name); } + template ConstMemberIterator FindMember(const GenericValue& name) const { return const_cast(*this).FindMember(name); } //! Add a member (name-value pair) to the object. /*! \param name A string value as name of member. @@ -971,7 +978,8 @@ public: return RemoveMember(n); } - bool RemoveMember(const GenericValue& name) { + template + bool RemoveMember(const GenericValue& name) { MemberIterator m = FindMember(name); if (m != MemberEnd()) { RemoveMember(m); @@ -1352,8 +1360,8 @@ int z = a[0u].GetInt(); // This works too. } private: - template - friend class GenericDocument; + template friend class GenericValue; + template friend class GenericDocument; enum { kBoolFlag = 0x100, @@ -1477,7 +1485,8 @@ private: rhs.flags_ = kNullFlag; } - bool StringEqual(const GenericValue& rhs) const { + template + bool StringEqual(const GenericValue& rhs) const { RAPIDJSON_ASSERT(IsString()); RAPIDJSON_ASSERT(rhs.IsString()); return data_.s.length == rhs.data_.s.length && @@ -1671,7 +1680,7 @@ private: // callers of the following private Handler functions template friend class GenericReader; // for parsing - friend class GenericValue; // for deep copying + template friend class GenericValue; // for deep copying // Implementation of Handler bool Null() { new (stack_.template Push()) ValueType(); return true; } From 1da078433111558ec3a9e7cbdb96f5f8fb26c436 Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sat, 30 Aug 2014 13:22:04 +0200 Subject: [PATCH 2/7] GenericValue: add and use IsGenericValue meta function This commit adds an IsGenericValue meta function to match arbitrary instantiations of the GenericValue template (or derived classes). This meta function is used in the SFINAE-checks to avoid matching the generic APIs (operator=,==,!=; AddMember, PushBack) for instances of the main template. This avoids ambiguities with the GenericValue overloads. --- include/rapidjson/document.h | 28 ++++++++++++++++++++++------ include/rapidjson/internal/meta.h | 2 ++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 8adfd7d..e206d84 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -382,6 +382,22 @@ inline GenericStringRef StringRef(const std::basic_string& s } #endif +/////////////////////////////////////////////////////////////////////////////// +// GenericValue type traits +namespace internal { + +template +struct IsGenericValueImpl : FalseType {}; + +// select candidates according to nested encoding and allocator types +template struct IsGenericValueImpl::Type, typename Void::Type> + : IsBaseOf, T>::Type {}; + +// helper to match arbitrary GenericValue instantiations, including derived classes +template struct IsGenericValue : IsGenericValueImpl::Type {}; + +} // namespace internal + /////////////////////////////////////////////////////////////////////////////// // GenericValue @@ -687,7 +703,7 @@ public: //! Equal-to operator with primitive types /*! \tparam T Either \ref Type, \c int, \c unsigned, \c int64_t, \c uint64_t, \c double, \c true, \c false */ - template RAPIDJSON_DISABLEIF_RETURN((internal::OrExpr,internal::IsBaseOf >), (bool)) operator==(const T& rhs) const { return *this == GenericValue(rhs); } + template RAPIDJSON_DISABLEIF_RETURN((internal::OrExpr,internal::IsGenericValue >), (bool)) operator==(const T& rhs) const { return *this == GenericValue(rhs); } //! Not-equal-to operator /*! \return !(*this == rhs) @@ -698,17 +714,17 @@ public: //! Not-equal-to operator with arbitrary types /*! \return !(*this == rhs) */ - template RAPIDJSON_DISABLEIF_RETURN((internal::IsBaseOf), (bool)) operator!=(const T& rhs) const { return !(*this == rhs); } + template RAPIDJSON_DISABLEIF_RETURN((internal::IsGenericValue), (bool)) operator!=(const T& rhs) const { return !(*this == rhs); } //! Equal-to operator with arbitrary types (symmetric version) /*! \return (rhs == lhs) */ - template friend RAPIDJSON_DISABLEIF_RETURN((internal::IsBaseOf), (bool)) operator==(const T& lhs, const GenericValue& rhs) { return rhs == lhs; } + template friend RAPIDJSON_DISABLEIF_RETURN((internal::IsGenericValue), (bool)) operator==(const T& lhs, const GenericValue& rhs) { return rhs == lhs; } //! Not-Equal-to operator with arbitrary types (symmetric version) /*! \return !(rhs == lhs) */ - template friend RAPIDJSON_DISABLEIF_RETURN((internal::IsBaseOf), (bool)) operator!=(const T& lhs, const GenericValue& rhs) { return !(rhs == lhs); } + template friend RAPIDJSON_DISABLEIF_RETURN((internal::IsGenericValue), (bool)) operator!=(const T& lhs, const GenericValue& rhs) { return !(rhs == lhs); } //@} //!@name Type @@ -949,7 +965,7 @@ public: \note Amortized Constant time complexity. */ template - RAPIDJSON_DISABLEIF_RETURN((internal::IsPointer), (GenericValue&)) + RAPIDJSON_DISABLEIF_RETURN((internal::OrExpr, internal::IsGenericValue >), (GenericValue&)) AddMember(StringRefType name, T value, Allocator& allocator) { GenericValue n(name); GenericValue v(value); @@ -1174,7 +1190,7 @@ int z = a[0u].GetInt(); // This works too. \note Amortized constant time complexity. */ template - RAPIDJSON_DISABLEIF_RETURN((internal::IsPointer), (GenericValue&)) + RAPIDJSON_DISABLEIF_RETURN((internal::OrExpr, internal::IsGenericValue >), (GenericValue&)) PushBack(T value, Allocator& allocator) { GenericValue v(value); return PushBack(v, allocator); diff --git a/include/rapidjson/internal/meta.h b/include/rapidjson/internal/meta.h index bcff1a5..578b670 100644 --- a/include/rapidjson/internal/meta.h +++ b/include/rapidjson/internal/meta.h @@ -38,6 +38,8 @@ RAPIDJSON_DIAG_OFF(6334) namespace rapidjson { namespace internal { +// Helper to wrap/convert arbitrary types to void, useful for arbitrary type matching +template struct Void { typedef void Type; }; /////////////////////////////////////////////////////////////////////////////// // BoolType, TrueType, FalseType From a40dcb95254a14d6117c5bcc0a9bbf58cee55ae8 Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sat, 30 Aug 2014 18:26:57 +0200 Subject: [PATCH 3/7] valuetest: always test comparisons with different allocators --- test/unittest/valuetest.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 9324813..d57f943 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -105,20 +105,12 @@ TEST(Value, equalto_operator) { TestEqual(x["i"], 123); TestEqual(x["pi"], 3.14); - // Test operator==() -#ifdef RAPIDJSON_COMPARE_DIFFERENT_ALLOCATORS + // Test operator==() (including different allocators) CrtAllocator crtAllocator; GenericValue, CrtAllocator> y; GenericDocument, CrtAllocator> z(&crtAllocator); - CrtAllocator& yAllocator = crtAllocator; -#else - Value::AllocatorType& yAllocator = allocator; - Value y; - Document z; -#endif // RAPIDJSON_COMPARE_DIFFERENT_ALLOCATORS - y.CopyFrom(x, yAllocator); + y.CopyFrom(x, crtAllocator); z.CopyFrom(y, z.GetAllocator()); - TestEqual(x, y); TestEqual(y, z); TestEqual(z, x); @@ -130,7 +122,7 @@ TEST(Value, equalto_operator) { EXPECT_TRUE(z.RemoveMember("t")); TestUnequal(x, z); TestEqual(y, z); - y.AddMember("t", true, yAllocator); + y.AddMember("t", true, crtAllocator); z.AddMember("t", true, z.GetAllocator()); TestEqual(x, y); TestEqual(y, z); From f8db47377916f66bfb899c56f71f9de275a5ab51 Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sat, 30 Aug 2014 18:51:43 +0200 Subject: [PATCH 4/7] GenericValue: add explicit operator!=(const Ch*) MSVC'2005 needs an explicit overload for operator!=(const Ch*) after the addition of the IsGenericValue SFINAE restrictions. --- include/rapidjson/document.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index e206d84..1eb506c 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -711,6 +711,9 @@ public: template bool operator!=(const GenericValue& rhs) const { return !(*this == rhs); } + //! Not-equal-to operator with const C-string pointer + bool operator!=(const Ch* rhs) const { return !(*this == rhs); } + //! Not-equal-to operator with arbitrary types /*! \return !(*this == rhs) */ From c59750576159dc11855ee684b732201a7f9f0dd3 Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sun, 31 Aug 2014 10:27:26 +0200 Subject: [PATCH 5/7] valuetest: test member operations with different allocators --- test/unittest/valuetest.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index d57f943..9b2658f 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -830,10 +830,18 @@ TEST(Value, Object) { EXPECT_TRUE(x.HasMember(name)); EXPECT_TRUE(y.HasMember(name)); + GenericValue, CrtAllocator> othername("A"); + EXPECT_TRUE(x.HasMember(othername)); + EXPECT_TRUE(y.HasMember(othername)); + othername.SetString("C\0D"); + EXPECT_TRUE(x.HasMember(othername)); + EXPECT_TRUE(y.HasMember(othername)); + // operator[] EXPECT_STREQ("Apple", x["A"].GetString()); EXPECT_STREQ("Banana", x["B"].GetString()); EXPECT_STREQ("CherryD", x[C0D].GetString()); + EXPECT_STREQ("CherryD", x[othername].GetString()); // const operator[] EXPECT_STREQ("Apple", y["A"].GetString()); @@ -904,7 +912,7 @@ TEST(Value, Object) { x.RemoveMember("B"); EXPECT_FALSE(x.HasMember("B")); - x.RemoveMember(name); + x.RemoveMember(othername); EXPECT_FALSE(x.HasMember(name)); EXPECT_TRUE(x.MemberBegin() == x.MemberEnd()); From ffed1d67c1c7747e4331010de23295bad2f1557b Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sun, 31 Aug 2014 10:29:59 +0200 Subject: [PATCH 6/7] valuetest: more testing of MemberCount --- test/unittest/valuetest.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 9b2658f..6705538 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -925,11 +925,14 @@ TEST(Value, Object) { for (int i = 0; i < 10; i++) x.AddMember(keys[i], Value(kArrayType).PushBack(i, allocator), allocator); + // MemberCount, iterator difference + EXPECT_EQ(x.MemberCount(), SizeType(x.MemberEnd() - x.MemberBegin())); + // Erase the first itr = x.EraseMember(x.MemberBegin()); EXPECT_FALSE(x.HasMember(keys[0])); EXPECT_EQ(x.MemberBegin(), itr); - EXPECT_EQ(9, x.MemberEnd() - x.MemberBegin()); + EXPECT_EQ(9u, x.MemberCount()); for (; itr != x.MemberEnd(); ++itr) { int i = (itr - x.MemberBegin()) + 1; EXPECT_STREQ(itr->name.GetString(), keys[i]); @@ -940,7 +943,7 @@ TEST(Value, Object) { itr = x.EraseMember(x.MemberEnd() - 1); EXPECT_FALSE(x.HasMember(keys[9])); EXPECT_EQ(x.MemberEnd(), itr); - EXPECT_EQ(8, x.MemberEnd() - x.MemberBegin()); + EXPECT_EQ(8u, x.MemberCount()); for (; itr != x.MemberEnd(); ++itr) { int i = (itr - x.MemberBegin()) + 1; EXPECT_STREQ(itr->name.GetString(), keys[i]); @@ -951,7 +954,7 @@ TEST(Value, Object) { itr = x.EraseMember(x.MemberBegin() + 4); EXPECT_FALSE(x.HasMember(keys[5])); EXPECT_EQ(x.MemberBegin() + 4, itr); - EXPECT_EQ(7, x.MemberEnd() - x.MemberBegin()); + EXPECT_EQ(7u, x.MemberCount()); for (; itr != x.MemberEnd(); ++itr) { int i = (itr - x.MemberBegin()); i += (i<4) ? 1 : 2; @@ -975,7 +978,7 @@ TEST(Value, Object) { EXPECT_EQ(x.MemberBegin() + first, itr); size_t removeCount = last - first; - EXPECT_EQ(n - removeCount, size_t(x.MemberEnd() - x.MemberBegin())); + EXPECT_EQ(n - removeCount, x.MemberCount()); for (unsigned i = 0; i < first; i++) EXPECT_EQ(i, x[keys[i]][0u].GetUint()); for (unsigned i = first; i < n - removeCount; i++) From a2a0d16167ffbb11ca83fdd2489b4ce21e4173fd Mon Sep 17 00:00:00 2001 From: "Philipp A. Hartmann" Date: Sun, 31 Aug 2014 11:08:33 +0200 Subject: [PATCH 7/7] unittest.h: simplify AssertException Some compilers warn about the missing initialisation of the std::exception base class of the AssertException helper. The simplest solution is to inherit from std::logic_error instead, which provides all of the required functionality already. --- test/unittest/unittest.h | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/unittest/unittest.h b/test/unittest/unittest.h index a476db7..6f5230a 100644 --- a/test/unittest/unittest.h +++ b/test/unittest/unittest.h @@ -89,15 +89,9 @@ inline FILE* TempFile(char *filename) { #pragma warning(disable : 4127) #endif -class AssertException : public std::exception { +class AssertException : public std::logic_error { public: - AssertException(const char* w) : what_(w) {} - AssertException(const AssertException& other) : what_(other.what_) {} - AssertException& operator=(const AssertException& rhs) { what_ = rhs.what_; return *this; } - virtual const char* what() const throw() { return what_; } - -private: - const char* what_; + AssertException(const char* w) : std::logic_error(w) {} }; #define RAPIDJSON_ASSERT(x) if (!(x)) throw AssertException(RAPIDJSON_STRINGIFY(x))