Tencent / rapidjson

A fast JSON parser/generator for C++ with both SAX/DOM style API
http://rapidjson.org/
Other
14.18k stars 3.53k forks source link

Copy Document #950

Open maaikez opened 7 years ago

maaikez commented 7 years ago

Hello,

I am trying to copy a document into another document (as sub-object), but I get assertion errors for IsString().

I want to 'serialize' some classes. Every class has it's own 'objectToJson' function. I don't make the json document on the fly, but only when I have time to make it. For that reason, I want to return a copy, so the whole document is not 'gone' before a new one is created. I merge the different json documents together so at the end I have one big document.

However, copying the Document is not working well. I try this: rapidjson::Document returnDoc; returnDoc.copyFrom(objectAsJson, returnDoc.GetAllocator()); return returnDoc;

Where objectAsJson is the rapidjson::Document I created before.

When copying like this, I get a IsString assertion error. I indeed use strings, but when copying another way, it works like expected: rapidjson::Value jsonValue = rapidjson::Value(jsonObject->GetObjectAsJson(), objectAsJson.GetAllocator());

However, this results in a rapidjson::Value and I want to return a rapidjson::Document. How can I achieve making a deep copy of a Document, so I can return a copy of a Document?

maaikez commented 7 years ago

Can there be a problem doing the 'copyFrom' more levels deep? It seems to work when I only do a deep copy on the highest level (not 100% sure), but when also adding a copy in a lower level, I get the assertion error.

pah commented 7 years ago

Can you show some more code that demonstrates the issue, including the failing assert? Ideally a self-contained example? The copying itself looks ok to me and it does support multiple levels of nested values.

maaikez commented 7 years ago

I uploaded an example. Unfortunately, it doesn't show the exact behaviour as I experienced in my project, but when "writing" with PrettyWriter to a buffer, I also get an IsString assertion error.

example.tar.gz

maaikez commented 7 years ago

Ok when changing this: rapidjson::Document marbleDocument = marbleBag.GetObjectAsJson(); marbleBags.PushBack(marbleDocument, objectAsJson.GetAllocator());

to this: rapidjson::Value marbleBagValue = rapidjson::Value(marbleBag.GetObjectAsJson(), objectAsJson.GetAllocator()); marbleBags.PushBack(marbleBagValue, objectAsJson.GetAllocator());

I don't get assertion errors. However, this way I make another copy?

And I still have troubles with my own code, I'm not sure how to reproduce it :(.

pah commented 7 years ago

RapidJSON uses move semantics by default (even before C++11), which is why you need a special syntax to actually make a copy.

In your failing snippet, you try to move marbeDocument into itself, where I indeed expect funny things to happen. Maybe we could add an assert to catch such "self moves" also for PushBack and AddMember.

Long story short: The second snippet is the correct solution and you still only have one copy.

maaikez commented 7 years ago

It looks like I found the problem: use the wrong allocator when copying.