1

First, the code...

#include <rapidjson/document.h>
#include <rapidjson/stringbuffer.h>
#include <rapidjson/writer.h>
#include <iostream>

int main(int argc, char* argv[]) {
    
    using namespace rapidjson;

    Value author;
    {
        Document document;
        char buffer[10];
        int len = sprintf(buffer, "%s %s", "Milo", "Yip"); // dynamically created string.
        author.SetString(buffer, len, document.GetAllocator());
    }

    // Checkpoint 1

    StringBuffer buffer;
    Writer<rapidjson::StringBuffer> writer( buffer );
    
    // Accesses invalidated memory in author
    author.Accept( writer );

    std::cout << buffer.GetString( ) << std::endl;

    return 0;
}

This code is a little contrived, and is a small variation on one of the examples from the docs. If I understand RapidJson correctly, the string inside author has been allocated using the allocator from document, and is owned by document. Consequently, when document drops out of scope, just before 'Checkpoint 1', author is invalidated.

I want be wrong about this, because if it's true it makes it more or less impossible to regard Value objects as first class objects, because they have a hidden dependency on their allocators, and to build Value objects piecemeal. Either you have to pass in an allocator to every function which builds a Value, or have one or more global allocators sitting around which you ensure remain in scope.

1 Answers1

2

As with any design decisions, there are reasons. From https://rapidjson.org/md_doc_tutorial.html:

To make memory allocation customizable, RapidJSON requires users to pass an instance of allocator, whenever an operation may require allocation. This design is needed to prevent storing an allocator (or Document) pointer per Value.

Now, whether this is a good or bad design - it depends. There are different programming paradigms and design patterns, hence it is important to look through specific lenses when analyzing the code.

The way I understand it, the lifetime of a Document should be longer than the one of a Value - I expect most users would have a document which would outlive all the values related to it.

Maxim Chetrusca
  • 3,262
  • 1
  • 32
  • 28
  • Hi Maxim, Thanks for that. I've understood that the allocation architecture is a reasonable design decision, at least superficially, until I realised the implications of that combined with move semantics. As far as I can see, it makes it impossible to write code like... `Document str_to_json(const char* json) { Document d; document.Parse(json); return std::move(d); } Document d(kArrayType); d.Pushback(str_to_json( "[1,2,3]").Move(), d.GetAllocator() ); ` – RobertGBJones Aug 16 '21 at 08:36
  • @RobertGBJones sorry for a delayed response, but: `return std::move(d);` is probably not what you want - compiler would do RVO without the move - https://stackoverflow.com/q/17473753/939050 – Maxim Chetrusca Aug 19 '21 at 18:50