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

Fix move constructor of json_ref #2405

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

karzhenkov
Copy link
Contributor

The defaulted move constructor of nlohmann::detail::json_ref makes copy of internal pointer which points to a field inside the object being moved. It becomes a dangling pointer. A "manual" move implementation is needed.

Resolves #2387. Here is an example at Godbolt.

@coveralls
Copy link

coveralls commented Sep 26, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 42a9dc0 on karzhenkov:fix-json_ref-move into fd7a9f6 on nlohmann:develop.

@karzhenkov
Copy link
Contributor Author

karzhenkov commented Sep 29, 2020

There is also a better solution: karzhenkov/json@42a9dc0.
No const_cast required; defaulted move constructor is pretty enough.

I'm ready to force-push if it's acceptable.

@nlohmann
Copy link
Owner

nlohmann commented Nov 7, 2020

@karzhenkov That would be great!

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann merged commit 3ad6992 into nlohmann:develop Dec 11, 2020
@nlohmann
Copy link
Owner

Thanks!

@nlohmann nlohmann self-assigned this Dec 11, 2020
@nlohmann nlohmann added this to the Release 3.9.2 milestone Dec 11, 2020
@Qix-
Copy link

Qix- commented Dec 11, 2020

Great catch @karzhenkov, didn't expect #2387 to be a bug.

@nlohmann
Copy link
Owner

Sorry everybody for waiting so long, and thanks for your patience!

@karzhenkov karzhenkov deleted the fix-json_ref-move branch December 12, 2020 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usage with -fno-elide-constructors causes dump() output to be array of nulls
4 participants