Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial C++11 move support #128

Merged
merged 12 commits into from
Sep 3, 2014
Merged
3 changes: 2 additions & 1 deletion build/Doxyfile
Original file line number Diff line number Diff line change
Expand Up @@ -2004,7 +2004,8 @@ PREDEFINED = \
# definition found in the source code.
# This tag requires that the tag ENABLE_PREPROCESSING is set to YES.

EXPAND_AS_DEFINED =
EXPAND_AS_DEFINED = \
RAPIDJSON_NOEXCEPT

# If the SKIP_FUNCTION_MACROS tag is set to YES then doxygen's preprocessor will
# remove all references to function-like macros that are alone on a line, have
Expand Down
81 changes: 60 additions & 21 deletions include/rapidjson/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
/*! \file document.h */

#include "reader.h"
#include "internal/meta.h"
#include "internal/strfunc.h"
#include <new> // placement new

Expand All @@ -38,15 +39,17 @@ RAPIDJSON_DIAG_OFF(effc++)
///////////////////////////////////////////////////////////////////////////////
// RAPIDJSON_HAS_STDSTRING

#ifndef RAPIDJSON_HAS_STDSTRING
#ifdef RAPIDJSON_DOXYGEN_RUNNING
#define RAPIDJSON_HAS_STDSTRING 1 // force generation of documentation
#else
#define RAPIDJSON_HAS_STDSTRING 0 // no std::string support by default
#endif
#ifdef RAPIDJSON_HAS_STDSTRING
/*! \def RAPIDJSON_HAS_STDSTRING
\ingroup RAPIDJSON_CONFIG
\brief Enable RapidJSON support for \c std::string

By defining this preprocessor symbol, several convenience functions for using
By defining this preprocessor symbol to \c 1, several convenience functions for using
\ref rapidjson::GenericValue with \c std::string are enabled, especially
for construction and comparison.

Expand All @@ -56,7 +59,6 @@ RAPIDJSON_DIAG_OFF(effc++)
#endif // RAPIDJSON_HAS_STDSTRING

#ifndef RAPIDJSON_NOMEMBERITERATORCLASS
#include "internal/meta.h"
#include <iterator> // std::iterator, std::random_access_iterator_tag
#endif

Expand Down Expand Up @@ -363,7 +365,7 @@ inline GenericStringRef<CharType> StringRef(const CharType* str, size_t length)
return GenericStringRef<CharType>(str, SizeType(length));
}

#ifdef RAPIDJSON_HAS_STDSTRING
#if RAPIDJSON_HAS_STDSTRING
//! Mark a string object as constant string
/*! Mark a string object (e.g. \c std::string) as a "string literal".
This function can be used to avoid copying a string to be referenced as a
Expand Down Expand Up @@ -414,7 +416,14 @@ class GenericValue {
//@{

//! Default constructor creates a null value.
GenericValue() : data_(), flags_(kNullFlag) {}
GenericValue() RAPIDJSON_NOEXCEPT : data_(), flags_(kNullFlag) {}

#if RAPIDJSON_HAS_CXX11_RVALUE_REFS
//! Move constructor in C++11
GenericValue(GenericValue&& rhs) RAPIDJSON_NOEXCEPT : data_(rhs.data_), flags_(rhs.flags_) {
rhs.flags_ = kNullFlag; // give up contents
}
#endif

private:
//! Copy constructor is not permitted.
Expand All @@ -427,7 +436,7 @@ class GenericValue {
\param type Type of the value.
\note Default content for number is zero.
*/
GenericValue(Type type) : data_(), flags_() {
GenericValue(Type type) RAPIDJSON_NOEXCEPT : data_(), flags_() {
static const unsigned defaultFlags[7] = {
kNullFlag, kFalseFlag, kTrueFlag, kObjectFlag, kArrayFlag, kConstStringFlag,
kNumberAnyFlag
Expand All @@ -454,31 +463,31 @@ class GenericValue {
*/
#ifndef RAPIDJSON_DOXYGEN_RUNNING // hide SFINAE from Doxygen
template <typename T>
explicit GenericValue(T b, RAPIDJSON_ENABLEIF((internal::IsSame<T,bool>)))
explicit GenericValue(T b, RAPIDJSON_ENABLEIF((internal::IsSame<T,bool>))) RAPIDJSON_NOEXCEPT
#else
explicit GenericValue(bool b)
explicit GenericValue(bool b) RAPIDJSON_NOEXCEPT
#endif
: data_(), flags_(b ? kTrueFlag : kFalseFlag) {
// safe-guard against failing SFINAE
RAPIDJSON_STATIC_ASSERT((internal::IsSame<bool,T>::Value));
}

//! Constructor for int value.
explicit GenericValue(int i) : data_(), flags_(kNumberIntFlag) {
explicit GenericValue(int i) RAPIDJSON_NOEXCEPT : data_(), flags_(kNumberIntFlag) {
data_.n.i64 = i;
if (i >= 0)
flags_ |= kUintFlag | kUint64Flag;
}

//! Constructor for unsigned value.
explicit GenericValue(unsigned u) : data_(), flags_(kNumberUintFlag) {
explicit GenericValue(unsigned u) RAPIDJSON_NOEXCEPT : data_(), flags_(kNumberUintFlag) {
data_.n.u64 = u;
if (!(u & 0x80000000))
flags_ |= kIntFlag | kInt64Flag;
}

//! Constructor for int64_t value.
explicit GenericValue(int64_t i64) : data_(), flags_(kNumberInt64Flag) {
explicit GenericValue(int64_t i64) RAPIDJSON_NOEXCEPT : data_(), flags_(kNumberInt64Flag) {
data_.n.i64 = i64;
if (i64 >= 0) {
flags_ |= kNumberUint64Flag;
Expand All @@ -492,7 +501,7 @@ class GenericValue {
}

//! Constructor for uint64_t value.
explicit GenericValue(uint64_t u64) : data_(), flags_(kNumberUint64Flag) {
explicit GenericValue(uint64_t u64) RAPIDJSON_NOEXCEPT : data_(), flags_(kNumberUint64Flag) {
data_.n.u64 = u64;
if (!(u64 & RAPIDJSON_UINT64_C2(0x80000000, 0x00000000)))
flags_ |= kInt64Flag;
Expand All @@ -503,21 +512,21 @@ class GenericValue {
}

//! Constructor for double value.
explicit GenericValue(double d) : data_(), flags_(kNumberDoubleFlag) { data_.n.d = d; }
explicit GenericValue(double d) RAPIDJSON_NOEXCEPT : data_(), flags_(kNumberDoubleFlag) { data_.n.d = d; }

//! Constructor for constant string (i.e. do not make a copy of string)
GenericValue(const Ch* s, SizeType length) : data_(), flags_() { SetStringRaw(StringRef(s, length)); }
GenericValue(const Ch* s, SizeType length) RAPIDJSON_NOEXCEPT : data_(), flags_() { SetStringRaw(StringRef(s, length)); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this is not noexcept, since it will/can involve a memory allocation...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it won't allocate memory. See SetStringRaw (and comment before the constructor).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mixed that with the version taking the allocator (since I only used that for now)...


//! Constructor for constant string (i.e. do not make a copy of string)
explicit GenericValue(StringRefType s) : data_(), flags_() { SetStringRaw(s); }
explicit GenericValue(StringRefType s) RAPIDJSON_NOEXCEPT : data_(), flags_() { SetStringRaw(s); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.


//! Constructor for copy-string (i.e. do make a copy of string)
GenericValue(const Ch* s, SizeType length, Allocator& allocator) : data_(), flags_() { SetStringRaw(StringRef(s, length), allocator); }

//! Constructor for copy-string (i.e. do make a copy of string)
GenericValue(const Ch*s, Allocator& allocator) : data_(), flags_() { SetStringRaw(StringRef(s), allocator); }

#ifdef RAPIDJSON_HAS_STDSTRING
#if RAPIDJSON_HAS_STDSTRING
//! Constructor for copy-string from a string object (i.e. do make a copy of string)
/*! \note Requires the definition of the preprocessor symbol \ref RAPIDJSON_HAS_STDSTRING.
*/
Expand Down Expand Up @@ -560,19 +569,26 @@ class GenericValue {
//! Assignment with move semantics.
/*! \param rhs Source of the assignment. It will become a null value after assignment.
*/
GenericValue& operator=(GenericValue& rhs) {
GenericValue& operator=(GenericValue& rhs) RAPIDJSON_NOEXCEPT {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, even lvalue assignment is always a move in RapidJSON, see Move Semantics.

RAPIDJSON_ASSERT(this != &rhs);
this->~GenericValue();
RawAssign(rhs);
return *this;
}

#if RAPIDJSON_HAS_CXX11_RVALUE_REFS
//! Move assignment in C++11
GenericValue& operator=(GenericValue&& rhs) RAPIDJSON_NOEXCEPT {
return *this = rhs.Move();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Move() should also be marked as noexcept

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

}
#endif

//! Assignment of constant string reference (no copy)
/*! \param str Constant string reference to be assigned
\note This overload is needed to avoid clashes with the generic primitive type assignment overload below.
\see GenericStringRef, operator=(T)
*/
GenericValue& operator=(StringRefType str) {
GenericValue& operator=(StringRefType str) RAPIDJSON_NOEXCEPT {
GenericValue s(str);
return *this = s;
}
Expand Down Expand Up @@ -615,7 +631,7 @@ class GenericValue {
\param other Another value.
\note Constant complexity.
*/
GenericValue& Swap(GenericValue& other) {
GenericValue& Swap(GenericValue& other) RAPIDJSON_NOEXCEPT {
GenericValue temp;
temp.RawAssign(*this);
RawAssign(other);
Expand Down Expand Up @@ -675,7 +691,7 @@ class GenericValue {
//! Equal-to operator with const C-string pointer
bool operator==(const Ch* rhs) const { return *this == GenericValue(StringRef(rhs)); }

#ifdef RAPIDJSON_HAS_STDSTRING
#if RAPIDJSON_HAS_STDSTRING
//! Equal-to operator with string object
/*! \note Requires the definition of the preprocessor symbol \ref RAPIDJSON_HAS_STDSTRING.
*/
Expand Down Expand Up @@ -895,6 +911,23 @@ class GenericValue {
return *this;
}

#if RAPIDJSON_HAS_CXX11_RVALUE_REFS
GenericValue& AddMember(GenericValue&& name, GenericValue&& value, Allocator& allocator) {
return AddMember(name, value, allocator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infinite recursion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, inside the function, the rvalue-refs are lvalues again. This will call the AddMember(GenericValue&, GenericValue&, Allocator&) overload.

}
GenericValue& AddMember(GenericValue&& name, GenericValue& value, Allocator& allocator) {
return AddMember(name, value, allocator);
}
GenericValue& AddMember(GenericValue& name, GenericValue&& value, Allocator& allocator) {
return AddMember(name, value, allocator);
}
GenericValue& AddMember(StringRefType name, GenericValue&& value, Allocator& allocator) {
GenericValue n(name);
return AddMember(n, value, allocator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::move(n)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Even the lvalue-overload will move n to the destination. Even if not, for a string value constructed from a StringRefType, a move would be the same as a copy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I need to get my head around the "non-standard" behavior of the value class... Sorry for that.

BTW, with respect to the factor of 2 in line 904 (my iPad doesnt allow to leave a comment there?) have a llok here: https://github.com/facebook/folly/blob/master/folly/docs/FBVector.md ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, thanks for the review. :-)

Wrt the growth factor (also applies to the "object capacity", see AddMember), it may be best to open a separate issue for the discussion. I tend to agree that a factor of 2 is a bad choice. We could do something like:

o.capacity += (o.capacity >> 1); // * 1.5

and

Reserve(data_.a.capacity == 0 ? kDefaultArrayCapacity : data_.a.capacity + (data_.a.capacity >> 1), allocator);

}
#endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS


//! Add a member (name-value pair) to the object.
/*! \param name A constant string reference as name of member.
\param value Value of any type.
Expand Down Expand Up @@ -1135,6 +1168,12 @@ int z = a[0u].GetInt(); // This works too.
return *this;
}

#if RAPIDJSON_HAS_CXX11_RVALUE_REFS
GenericValue& PushBack(GenericValue&& value, Allocator& allocator) {
return PushBack(value, allocator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infinite recursion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, see above. value is an lvalue here and this will call PushBack(GenericValue&, Allocator&).

}
#endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS

//! Append a constant string reference at the end of the array.
/*! \param value Constant string reference to be appended.
\param allocator Allocator for reallocating memory. It must be the same one used previously. Commonly use GenericDocument::GetAllocator().
Expand Down Expand Up @@ -1289,7 +1328,7 @@ int z = a[0u].GetInt(); // This works too.
*/
GenericValue& SetString(const Ch* s, Allocator& allocator) { return SetString(s, internal::StrLen(s), allocator); }

#ifdef RAPIDJSON_HAS_STDSTRING
#if RAPIDJSON_HAS_STDSTRING
//! Set this value as a string by copying from source string.
/*! \param s source string.
\param allocator Allocator for allocating copied buffer. Commonly use GenericDocument::GetAllocator().
Expand Down
8 changes: 6 additions & 2 deletions include/rapidjson/internal/meta.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
#ifndef RAPIDJSON_INTERNAL_META_H_
#define RAPIDJSON_INTERNAL_META_H_

#ifndef RAPIDJSON_RAPIDJSON_H_
#error <rapidjson.h> not yet included. Do not include this file directly.
#endif

#ifdef __GNUC__
RAPIDJSON_DIAG_PUSH
RAPIDJSON_DIAG_OFF(effc++)
Expand All @@ -30,7 +34,7 @@ RAPIDJSON_DIAG_PUSH
RAPIDJSON_DIAG_OFF(6334)
#endif

#ifdef RAPIDJSON_HAS_CXX11_TYPETRAITS
#if RAPIDJSON_HAS_CXX11_TYPETRAITS
#include <type_traits>
#endif

Expand Down Expand Up @@ -98,7 +102,7 @@ template <typename T> struct IsPointer<T*> : TrueType {};
///////////////////////////////////////////////////////////////////////////////
// IsBaseOf
//
#ifdef RAPIDJSON_HAS_CXX11_TYPETRAITS
#if RAPIDJSON_HAS_CXX11_TYPETRAITS

template <typename B, typename D> struct IsBaseOf
: BoolType< ::std::is_base_of<B,D>::value> {};
Expand Down
45 changes: 43 additions & 2 deletions include/rapidjson/rapidjson.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,20 @@ template<int x> struct StaticAssertTest {};
///////////////////////////////////////////////////////////////////////////////
// RAPIDJSON_DIAG_PUSH/POP, RAPIDJSON_DIAG_OFF

#if defined(__clang__) || (defined(__GNUC__) && RAPIDJSON_VERSION_CODE(__GNUC__,__GNUC_MINOR__,__GNUC_PATCHLEVEL__) >= RAPIDJSON_VERSION_CODE(4,2,0))
#if defined(__GNUC__)
#define RAPIDJSON_GNUC \
RAPIDJSON_VERSION_CODE(__GNUC__,__GNUC_MINOR__,__GNUC_PATCHLEVEL__)
#endif

#if defined(__clang__) || (defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,2,0))

#define RAPIDJSON_PRAGMA(x) _Pragma(RAPIDJSON_STRINGIFY(x))
#define RAPIDJSON_DIAG_PRAGMA(x) RAPIDJSON_PRAGMA(GCC diagnostic x)
#define RAPIDJSON_DIAG_OFF(x) \
RAPIDJSON_DIAG_PRAGMA(ignored RAPIDJSON_STRINGIFY(RAPIDJSON_JOIN(-W,x)))

// push/pop support in Clang and GCC>=4.6
#if defined(__clang__) || (defined(__GNUC__) && RAPIDJSON_VERSION_CODE(__GNUC__,__GNUC_MINOR__,__GNUC_PATCHLEVEL__) >= RAPIDJSON_VERSION_CODE(4,6,0))
#if defined(__clang__) || (defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,6,0))
#define RAPIDJSON_DIAG_PUSH RAPIDJSON_DIAG_PRAGMA(push)
#define RAPIDJSON_DIAG_POP RAPIDJSON_DIAG_PRAGMA(pop)
#else // GCC >= 4.2, < 4.6
Expand All @@ -352,6 +357,42 @@ template<int x> struct StaticAssertTest {};

#endif // RAPIDJSON_DIAG_*

///////////////////////////////////////////////////////////////////////////////
// C++11 features

#ifndef RAPIDJSON_HAS_CXX11_RVALUE_REFS
#if defined(__clang__)
#define RAPIDJSON_HAS_CXX11_RVALUE_REFS __has_feature(cxx_rvalue_references)
#elif (defined(RAPIDJSON_GNUC) && (RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,3,0)) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || \
(defined(_MSC_VER) && _MSC_VER >= 1600)

#define RAPIDJSON_HAS_CXX11_RVALUE_REFS 1
#else
#define RAPIDJSON_HAS_CXX11_RVALUE_REFS 0
#endif
#endif // RAPIDJSON_HAS_CXX11_RVALUE_REFS

#ifndef RAPIDJSON_HAS_CXX11_NOEXCEPT
#if defined(__clang__)
#define RAPIDJSON_HAS_CXX11_NOEXCEPT __has_feature(cxx_noexcept)
#elif (defined(RAPIDJSON_GNUC) && (RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,6,0)) && defined(__GXX_EXPERIMENTAL_CXX0X__))
// (defined(_MSC_VER) && _MSC_VER >= ????) // not yet supported
#define RAPIDJSON_HAS_CXX11_NOEXCEPT 1
#else
#define RAPIDJSON_HAS_CXX11_NOEXCEPT 0
#endif
#endif
#if RAPIDJSON_HAS_CXX11_NOEXCEPT
#define RAPIDJSON_NOEXCEPT noexcept
#else
#define RAPIDJSON_NOEXCEPT /* noexcept */
#endif // RAPIDJSON_HAS_CXX11_NOEXCEPT

// no automatic detection, yet
#ifndef RAPIDJSON_HAS_CXX11_TYPETRAITS
#define RAPIDJSON_HAS_CXX11_TYPETRAITS 0
#endif

//!@endcond

///////////////////////////////////////////////////////////////////////////////
Expand Down
Loading