From 6e2e5c7dbe08474249ca18b50da120b2c45ccc36 Mon Sep 17 00:00:00 2001 From: StilesCrisis Date: Tue, 28 Feb 2017 01:11:30 -0800 Subject: [PATCH 01/12] Specialize StrLen for char/wchar_t Compilers generally provide a much smarter strlen than ours. --- include/rapidjson/internal/strfunc.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/rapidjson/internal/strfunc.h b/include/rapidjson/internal/strfunc.h index de41d8f..226439a 100644 --- a/include/rapidjson/internal/strfunc.h +++ b/include/rapidjson/internal/strfunc.h @@ -16,6 +16,7 @@ #define RAPIDJSON_INTERNAL_STRFUNC_H_ #include "../stream.h" +#include RAPIDJSON_NAMESPACE_BEGIN namespace internal { @@ -34,6 +35,16 @@ inline SizeType StrLen(const Ch* s) { return SizeType(p - s); } +template <> +inline SizeType StrLen(const char* s) { + return SizeType(std::strlen(s)); +} + +template <> +inline SizeType StrLen(const wchar_t* s) { + return SizeType(std::wcslen(s)); +} + //! Returns number of code points in a encoded string. template bool CountStringCodePoint(const typename Encoding::Ch* s, SizeType length, SizeType* outCount) { From 4b822a41af98974a4ca96b79cd13afe163214d81 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Tue, 28 Feb 2017 19:31:21 -0800 Subject: [PATCH 02/12] Attempt to suppress valgrind wcslen error --- test/unittest/CMakeLists.txt | 2 +- test/valgrind.supp | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 test/valgrind.supp diff --git a/test/unittest/CMakeLists.txt b/test/unittest/CMakeLists.txt index 4e29765..fdf0ad0 100644 --- a/test/unittest/CMakeLists.txt +++ b/test/unittest/CMakeLists.txt @@ -79,7 +79,7 @@ add_test(NAME unittest if(NOT MSVC) # Not running SIMD.* unit test cases for Valgrind add_test(NAME valgrind_unittest - COMMAND valgrind --leak-check=full --error-exitcode=1 ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittest --gtest_filter=-SIMD.* + COMMAND valgrind --suppressions=${CMAKE_SOURCE_DIR}/test/valgrind.supp --leak-check=full --error-exitcode=1 ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittest --gtest_filter=-SIMD.* WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/bin) if(CMAKE_BUILD_TYPE STREQUAL "Debug") diff --git a/test/valgrind.supp b/test/valgrind.supp new file mode 100644 index 0000000..5a205b7 --- /dev/null +++ b/test/valgrind.supp @@ -0,0 +1,5 @@ +{ + Suppress wcslen valgrind report + Memcheck:Addr8 + fun:__wcslen_sse2 +} From 13e99d8d5ffea627a169734128d38a31a7895b29 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Tue, 28 Feb 2017 22:58:24 -0800 Subject: [PATCH 03/12] Trivial change to re-trigger Travis CI No-op blank line --- test/unittest/writertest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unittest/writertest.cpp b/test/unittest/writertest.cpp index 398a63d..8fd6eb8 100644 --- a/test/unittest/writertest.cpp +++ b/test/unittest/writertest.cpp @@ -544,3 +544,4 @@ TEST(Writer, MoveCtor) { #ifdef __clang__ RAPIDJSON_DIAG_POP #endif + From 3f9ebfe9e9e9b4ac57f8d58fbce390b4f9a5977e Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 2 Mar 2017 21:24:03 -0800 Subject: [PATCH 04/12] Trivial change to trigger Travis CI --- include/rapidjson/writer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 43ec5dc..12d3145 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -623,3 +623,4 @@ RAPIDJSON_DIAG_POP #endif #endif // RAPIDJSON_RAPIDJSON_H_ + From 534f1352619328668e8bc4ffaf9bacb5de697180 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 3 Mar 2017 00:21:10 -0800 Subject: [PATCH 05/12] Try again to suppress Valgrind --- test/valgrind.supp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/valgrind.supp b/test/valgrind.supp index 5a205b7..8552385 100644 --- a/test/valgrind.supp +++ b/test/valgrind.supp @@ -1,5 +1,5 @@ { Suppress wcslen valgrind report - Memcheck:Addr8 + Memcheck:Cond fun:__wcslen_sse2 } From 6ae50ad6e32030c2d930b313a1c740acbbb0cca6 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 3 Mar 2017 00:27:47 -0800 Subject: [PATCH 06/12] Once again --- test/valgrind.supp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/valgrind.supp b/test/valgrind.supp index 8552385..1fed18b 100644 --- a/test/valgrind.supp +++ b/test/valgrind.supp @@ -1,5 +1,17 @@ { - Suppress wcslen valgrind report + Suppress wcslen valgrind report 1 Memcheck:Cond fun:__wcslen_sse2 } + +{ + Suppress wcslen valgrind report 2 + Memcheck:Addr8 + fun:__wcslen_sse2 +} + +{ + Suppress wcslen valgrind report 3 + Memcheck:Value8 + fun:__wcslen_sse2 +} From db8d3bb4d60aa20c5d6ec4be1f6e71c33d9874b9 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 3 Mar 2017 00:42:00 -0800 Subject: [PATCH 07/12] Remove unneeded change --- include/rapidjson/writer.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 12d3145..43ec5dc 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -623,4 +623,3 @@ RAPIDJSON_DIAG_POP #endif #endif // RAPIDJSON_RAPIDJSON_H_ - From 66b564f385d96d7adbe1ba3073a140f5301e0af6 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Fri, 3 Mar 2017 00:42:21 -0800 Subject: [PATCH 08/12] Remove unneeded change --- test/unittest/writertest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unittest/writertest.cpp b/test/unittest/writertest.cpp index 8fd6eb8..398a63d 100644 --- a/test/unittest/writertest.cpp +++ b/test/unittest/writertest.cpp @@ -544,4 +544,3 @@ TEST(Writer, MoveCtor) { #ifdef __clang__ RAPIDJSON_DIAG_POP #endif - From dd97ede84d517e2a1d433c5bfc1610d5d769a430 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Sun, 5 Mar 2017 00:27:08 -0800 Subject: [PATCH 09/12] Quoted strings to String() or Key() are auto-sized by template No strlen call needs to be made when templates can auto-deduce the string length. No strlen = faster! Unfortunately this needs a touch of SFINAE to allow multiple overrides to coexist cleanly. --- include/rapidjson/writer.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 43ec5dc..755f483 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -16,6 +16,7 @@ #define RAPIDJSON_WRITER_H_ #include "stream.h" +#include "internal/meta.h" #include "internal/stack.h" #include "internal/strfunc.h" #include "internal/dtoa.h" @@ -198,7 +199,8 @@ public: return EndValue(WriteString(str, length)); } - bool String(const Ch* str, SizeType length, bool copy = false) { + template + bool String(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { RAPIDJSON_ASSERT(str != 0); (void)copy; Prefix(kStringType); @@ -217,7 +219,8 @@ public: return WriteStartObject(); } - bool Key(const Ch* str, SizeType length, bool copy = false) { return String(str, length, copy); } + template + bool Key(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, length, copy); } bool EndObject(SizeType memberCount = 0) { (void)memberCount; @@ -247,8 +250,16 @@ public: //@{ //! Simpler but slower overload. - bool String(const Ch* str) { return String(str, internal::StrLen(str)); } - bool Key(const Ch* str) { return Key(str, internal::StrLen(str)); } + template + bool String(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, internal::StrLen(str)); } + template + bool Key(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, internal::StrLen(str)); } + + //! The compiler can give us the length of quoted strings for free. + template + bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, N-1); } + template + bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, N-1); } //@} From 61f8c4ef0df9d91fe6a684bb1b1572e0a537f66e Mon Sep 17 00:00:00 2001 From: John Stiles Date: Sun, 5 Mar 2017 00:38:34 -0800 Subject: [PATCH 10/12] Quoted strings to String() or Key() are auto-sized by template Same fix as previous commit, to prettywriter --- include/rapidjson/prettywriter.h | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/prettywriter.h b/include/rapidjson/prettywriter.h index b68b687..64301b8 100644 --- a/include/rapidjson/prettywriter.h +++ b/include/rapidjson/prettywriter.h @@ -107,7 +107,8 @@ public: return Base::WriteString(str, length); } - bool String(const Ch* str, SizeType length, bool copy = false) { + template + bool String(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { RAPIDJSON_ASSERT(str != 0); (void)copy; PrettyPrefix(kStringType); @@ -126,7 +127,8 @@ public: return Base::WriteStartObject(); } - bool Key(const Ch* str, SizeType length, bool copy = false) { return String(str, length, copy); } + template + bool Key(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, length, copy); } #if RAPIDJSON_HAS_STDSTRING bool Key(const std::basic_string& str) { @@ -184,8 +186,16 @@ public: //@{ //! Simpler but slower overload. - bool String(const Ch* str) { return String(str, internal::StrLen(str)); } - bool Key(const Ch* str) { return Key(str, internal::StrLen(str)); } + template + bool String(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, internal::StrLen(str)); } + template + bool Key(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, internal::StrLen(str)); } + + //! The compiler can give us the length of quoted strings for free. + template + bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, N-1); } + template + bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, N-1); } //@} From cdea825a0bc81531d2cd2758362b606106373477 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Sun, 5 Mar 2017 09:23:03 -0800 Subject: [PATCH 11/12] Assert that String() and Key() are given null-terminated strings Assert in case users attempt to pass a char array to String() or Key() that is not null terminated; that is not the intended use of the API. Null terminate your string buffers. --- include/rapidjson/prettywriter.h | 10 ++++++++-- include/rapidjson/writer.h | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/prettywriter.h b/include/rapidjson/prettywriter.h index 64301b8..abea404 100644 --- a/include/rapidjson/prettywriter.h +++ b/include/rapidjson/prettywriter.h @@ -193,9 +193,15 @@ public: //! The compiler can give us the length of quoted strings for free. template - bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, N-1); } + bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) + return String(str, N-1); + } template - bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, N-1); } + bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) + return Key(str, N-1); + } //@} diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index 755f483..c438f71 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -257,9 +257,15 @@ public: //! The compiler can give us the length of quoted strings for free. template - bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, N-1); } + bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) + return String(str, N-1); + } template - bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, N-1); } + bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) + return Key(str, N-1); + } //@} From c4e3d6243ce0321b32c9bfc7e3692753b21e46f8 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Sun, 5 Mar 2017 09:50:03 -0800 Subject: [PATCH 12/12] Fix msvc x64 compilation issue Disambiguate by putting the ENABLEIF on the return value instead of in the argument list. --- include/rapidjson/prettywriter.h | 12 ++++++------ include/rapidjson/writer.h | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/rapidjson/prettywriter.h b/include/rapidjson/prettywriter.h index abea404..a9d0f02 100644 --- a/include/rapidjson/prettywriter.h +++ b/include/rapidjson/prettywriter.h @@ -108,7 +108,7 @@ public: } template - bool String(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T* str, SizeType length, bool copy = false) { RAPIDJSON_ASSERT(str != 0); (void)copy; PrettyPrefix(kStringType); @@ -128,7 +128,7 @@ public: } template - bool Key(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, length, copy); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T* str, SizeType length, bool copy = false) { return String(str, length, copy); } #if RAPIDJSON_HAS_STDSTRING bool Key(const std::basic_string& str) { @@ -187,18 +187,18 @@ public: //! Simpler but slower overload. template - bool String(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, internal::StrLen(str)); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T* const& str) { return String(str, internal::StrLen(str)); } template - bool Key(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, internal::StrLen(str)); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T* const& str) { return Key(str, internal::StrLen(str)); } //! The compiler can give us the length of quoted strings for free. template - bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T (&str)[N]) { RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) return String(str, N-1); } template - bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T (&str)[N]) { RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) return Key(str, N-1); } diff --git a/include/rapidjson/writer.h b/include/rapidjson/writer.h index c438f71..7a0af39 100644 --- a/include/rapidjson/writer.h +++ b/include/rapidjson/writer.h @@ -200,7 +200,7 @@ public: } template - bool String(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T* str, SizeType length, bool copy = false) { RAPIDJSON_ASSERT(str != 0); (void)copy; Prefix(kStringType); @@ -220,7 +220,7 @@ public: } template - bool Key(const T* str, SizeType length, bool copy = false, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, length, copy); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T* str, SizeType length, bool copy = false) { return String(str, length, copy); } bool EndObject(SizeType memberCount = 0) { (void)memberCount; @@ -251,18 +251,18 @@ public: //! Simpler but slower overload. template - bool String(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return String(str, internal::StrLen(str)); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T* const& str) { return String(str, internal::StrLen(str)); } template - bool Key(const T* const& str, RAPIDJSON_ENABLEIF((internal::IsSame))) { return Key(str, internal::StrLen(str)); } + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T* const& str) { return Key(str, internal::StrLen(str)); } //! The compiler can give us the length of quoted strings for free. template - bool String(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) String(const T (&str)[N]) { RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) return String(str, N-1); } template - bool Key(const T (&str)[N], RAPIDJSON_ENABLEIF((internal::IsSame))) { + RAPIDJSON_ENABLEIF_RETURN((internal::IsSame), (bool)) Key(const T (&str)[N]) { RAPIDJSON_ASSERT(str[N-1] == '\0'); // you must pass in a null-terminated string (quoted constant strings are always null-terminated) return Key(str, N-1); }