Skip to content

Add assertion to converting constructor#3517

Merged
nlohmann merged 1 commit into
nlohmann:developfrom
falbrechtskirchinger:assert-conv-value_t
Jun 3, 2022
Merged

Add assertion to converting constructor#3517
nlohmann merged 1 commit into
nlohmann:developfrom
falbrechtskirchinger:assert-conv-value_t

Conversation

@falbrechtskirchinger
Copy link
Copy Markdown
Contributor

@falbrechtskirchinger falbrechtskirchinger commented Jun 3, 2022

The converting basic_json constructor can inadvertently change the value type of its parameter. Assert that both basic_json values are of the same value type after conversion.

This is not a fix for #3425, but instead of surprising the user with strings converting to arrays, etc., those conversions will at least fail.

The converting basic_json constructor can inadvertently change the value
type of its parameter. Assert that both basic_json values are of the
same value type after conversion.
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 4b75ae6 on falbrechtskirchinger:assert-conv-value_t into 6058d9a on nlohmann:develop.

Copy link
Copy Markdown
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.

Comment thread include/nlohmann/json.hpp
@nlohmann
Copy link
Copy Markdown
Owner

nlohmann commented Jun 3, 2022

Please change the wording about fixing: GitHub does not understand the NOT and would close #3425 when merging.

@falbrechtskirchinger
Copy link
Copy Markdown
Contributor Author

falbrechtskirchinger commented Jun 3, 2022

@nlohmann Done.

Edit: But you might want to wait. I think I'm close to a proper solution to #3425 that's quite a bit simpler than my previous attempt. 🤞

@nlohmann nlohmann self-assigned this Jun 3, 2022
@nlohmann nlohmann added this to the Release 3.11.0 milestone Jun 3, 2022
@nlohmann nlohmann merged commit 7a6e28a into nlohmann:develop Jun 3, 2022
@falbrechtskirchinger falbrechtskirchinger deleted the assert-conv-value_t branch June 3, 2022 19:06
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.

4 participants