-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Make serializer dump() non-recursive #4439
base: develop
Are you sure you want to change the base?
Conversation
Unfortunately has some compile errors on MSVC etc. Can update and fix if you like this direction -
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not saying whether this is something that should be pursued or not, but in case it is, a couple initial observations.
if (pretty_print) | ||
while (!stack.empty()) | ||
{ | ||
const dump_state& state = stack.back(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By taking a reference here, you end up with a dangling reference when you do pop_back()
. Even if you don't use state
again after the pop_back()
, you are referring to the BasicJsonType
inside it with val
. It's probably better to store a BasicJsonType *
in the state, so you don't have to worry about referenceds to references, and then you can do this:
const BasicJsonType* val = stack.back().val;
const unsigned int current_indent = stack.back().new_indent;
const unsigned int offset = stack.back().offset;
auto iterator = stack.back().iterator;
stack.pop_back();
const unsigned int offset; | ||
const decltype(start_val.m_data.m_value.object->cbegin()) iterator; | ||
}; | ||
std::vector<dump_state> stack; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A std::stack
would probably be a better choice.
9459221
to
6fdba5b
Compare
I very much like this fix here! It was on my todo list as well. |
This allows printing very deep trees without running out of stack.