search

pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
463 stars 133 forks source link

Fix bug: attr.value() of an array attribute returns an iterator of basic_json objects not an iterator of std::string #276

Closed VolkerChristian closed 10 months ago

VolkerChristian commented 10 months ago

This fixes also issue https://github.com/pboettch/json-schema-validator/issues/273.

pboettch commented 10 months ago

Should we add a new json version to the CI?

VolkerChristian commented 10 months ago

Sorry, i din't get the question.

pboettch commented 10 months ago

Does this problem is coming from new nlohmann::json version or from a new GCC version? In any case, this needs to be tested with an github-action. Would you be able to update your PR so that it tests the new version, respectively?

VolkerChristian commented 10 months ago

OK, now I understand. This problem is due to a new GCC version 13.1.0 and it obviously manifests itself also when using the latest MSVC 17.6.2. BTW I am using nlohman-json version 3.11.2 (latest official release from Aug 12, 2022). I think the compilers have become stricter with respect to template argument deduction. To be honest, I have not enough time to look deeper at what changes in 13.1.0 triggers this bug now. This does not happen with gcc version 12.3.0 and clang version 14.0.6. I just tested with the latest stable release of clang (16.0.6) which also didn't trigger that bug.

But do you really need a test for that? And what explicitly should be tested?

As the documentation of nlohman::json explicitly noted, that array.value() returns an iterator which can be used in a range based for loop to access all json-objects (values) of the array. And because the concrete array is an array of json type names (as std::strings) a concrete type name has to be accessed using .get<std::string>(). And important: No compiler has problems with that fix.

pboettch commented 10 months ago

You're right, my additional request has nothing to with this fix. It would have just been nice having a test for that new compiler version as well.

xamix commented 10 months ago

The bug raised with Clang-16.0.6 for me:

(Ubuntu clang version 16.0.6 (++20230610113410+7cbf1a259152-1~exp1~20230610233423.102)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Not with Clang-15:

Ubuntu clang version 15.0.7
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Anyway I will test this fix Regards.