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

Improved customization of RAPIDJSON_NEW macro #811

Merged
merged 3 commits into from
Dec 23, 2016

Conversation

glebov-andrey
Copy link

Reason for change
I am currently integrating RapidJSON into a large project with high performance requirements where custom allocation support is a must. Allocation is done via a custom scalable_alloc() function, so the syntax differs from the new expression (as does most any custom allocation function's syntax).
So, the object initialization should be new(scalable_alloc(sizeof(Type)) Type(Args...). But the macro's argument x supplies the expression Type(Args...). Thankfully, all uses of RAPIDJSON_NEW in the library have the form RAPIDJSON_NEW(AllocatorType()). This allowed me to hack together a custom type trait that extracts the Type part of Type() expressions with empty parentheses (works because the expression Type() is also a function signature):

template<typename> struct return_type;
template<typename R> struct return_type<R()> { using type = R; };
template<typename T> using return_type_t = typename return_type<T>::type;

Then the macro reads something like: new(scalable_alloc(sizeof(return_type_t<x>)) x which works if default initialization is used but breaks in any other case.

Proposed solution
Change the RAPIDJSON_NEW macro signature to RAPIDJSON_NEW(type, ...) and the default implementation to new type(__VA_ARGS__) as is done in this pull request.
This will allow custom allocation functions to be used without type trait hacking consistently with any amount of constructor arguments, e.g. new(scalable_alloc(sizeof(type))) type(__VA_ARGS__).

Committed code tested on Windows 7 with Visual Studio 2012 (64 bit).

@coveralls
Copy link

coveralls commented Dec 21, 2016

Coverage Status

Coverage remained the same at 99.938% when pulling 3f120ca on glebov-andrey:improved_new_macro into 014097e on miloyip:master.

@glebov-andrey
Copy link
Author

So, since __VA_ARGS__ isn't standard in C++98, there is another less versatile but more compatible solution (41ceb86)
We assume that any custom allocation + construction function may be converted to the form: custom_new_func(<forwarded c-tor arguments>) e.g. with placement new: new(scalable_alloc(sizeof(Type))) Type(Args...).
Since that last (Args...) part may be added outside of the actual macro, the macro only requires Type as a parameter. Then the default implementation will read: RAPIDJSON_NEW(Type) new Type and the user code: RAPIDJSON_NEW(AllocatorType)(<params>).
Using custom allocation will then take the form: RAPIDJSON_NEW(Type) new(scalable_alloc(sizeof(Type))) Type.
The only issue with this solution is that types that are template instantiations with commas in the template parameter list (e.g. Transcoder<UTF8<>, UTF8<> > from fwdtest.h) will have to be aliased before passing to the RAPIDJSON_NEW (e.g. typedef Transcoder<UTF8<>, UTF8<> > TranscoderUtf8ToUtf8).

@coveralls
Copy link

coveralls commented Dec 21, 2016

Coverage Status

Coverage remained the same at 99.938% when pulling 41ceb86 on glebov-andrey:improved_new_macro into 014097e on miloyip:master.

Copy link
Contributor

@pah pah left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. 👍

@miloyip miloyip merged commit 369e07d into Tencent:master Dec 23, 2016
@glebov-andrey glebov-andrey deleted the improved_new_macro branch December 26, 2016 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants