Allow preventing implicit converions to basic_json #4324
Conversation
Previously, `JSON_USE_IMPLICIT_CONVERSIONS` only prevented implicit conversions _from_ `basic_json` to other types. With this change, implicit conversions _to_ `basic_json` are also prevented by that macro.
|
As I thought, this will not work easily because a lot of code depends on implicit construction. |
|
Ah yes, it seems that the CI is running more tests that I didn't run locally. I'll have a look at that. |
|
Could we use a new define |
|
Good idea. |
|
So, after playing whack-a-mole with the tests for 2 hours, I think just making the constructor explicit is not enough, it breaks an incredible amount of features, including safe ones (see the latest WIP commit). A lot of functions already rely on the implicit conversion, so it seems necessary to add overloads for Maybe there's a smarter way to prevent nested temporary values from being implicitly converted? Maybe if they're required to be moved instead of allowing const references? |
| @@ -491,6 +491,20 @@ add_custom_target(ci_test_noimplicitconversions | |||
| COMMENT "Compile and test with implicit conversions switched off" | |||
| ) | |||
|
|
|||
| ############################################################################### | |||
| # Disable implicit converstructors. | |||
|
|
||
| ??? example | ||
|
|
||
| Disabling constructors is particularly useful to avoid implicit allocations that can lead to invalid memory access: |
There was a problem hiding this comment.
Disabling implicit constructors
|
I will not continuing work on this. If someone else is interested in picking it up, feel free to do so. |
Previously,
JSON_USE_IMPLICIT_CONVERSIONSonly prevented implicit conversions frombasic_jsonto other types.With this change, implicit conversions to
basic_jsonare also prevented by that macro.Fixes #2226.
I tested this change also in iFreilicht/json-questions@c0b2f53, the original repro repo from #2226 (comment).
Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmanndirectory, runmake amalgamateto create the single-header filessingle_include/nlohmann/json.hppandsingle_include/nlohmann/json_fwd.hpp. The whole process is described here.