Added to_string and added basic tests #1585
Conversation
|
@theodelrieu I’ll change it accordingly but I used the `std::to_string` because to the best of my knowledge `to_string` can’t be overwritten or specialized so that’s how you define the ADL. I could easily just use the `nlohmann::to_string` but I thought that we wanted this to allow input into `std::to_string`.
… On Apr 26, 2019, at 03:33, Théo DELRIEU ***@***.***> wrote:
@theodelrieu commented on this pull request.
In test/src/unit-serialization.cpp:
> @@ -173,4 +173,19 @@ TEST_CASE("serialization")
test("\xE1\x80\xE2\xF0\x91\x92\xF1\xBF\x41", "\\ufffd" "\\ufffd" "\\ufffd" "\\ufffd" "\x41");
}
}
+
+ SECTION("to_string")
+ {
+ auto test = [&](std::string const & input, std::string const & expected)
+ {
+ using std::to_string;
You should not have to use that for that specific test. The using std::thing is only necessary in generic code.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
That's not exactly it. The details are explained in this paper, but here's a short exlanation: There is a flaw in the design of customization points. The wanted behavior would be to always call This will be fixed in C++20 with Meanwhile, we must rely on using unqualified calls: // Pre C++20
template <typename T> void f(T const& container) {
using std::begin;
// If a free function begin exists in T's namespace, it will be picked up
// If not, std::begin will be used as a fallback
auto it = begin(container);
}
// C++20
template <typename T> void f(T const& container) {
// ranges::begin has the same behavior than the pre-C++20 mechanism.
auto it = ranges::begin(container);
}Anyway, the |
|
Ah alright thank you. I’ll change the test statements, but I’m going to leave `to_string` as is (except for changing to the ugly macro).
… On Apr 26, 2019, at 07:20, Théo DELRIEU ***@***.***> wrote:
to the best of my knowledge to_string can’t be overwritten or specialized so that’s how you define the ADL.
That's not exactly it. The details are explained in this paper, but here's a short exlanation:
There is a flaw in the design of customization points. The wanted behavior would be to always call std::to_string, which would pick ADL customization points, if any, and then consider the overloads in std.
This will be fixed in C++20 with ranges::begin, ranges::end, etc... Note that nlohmann::to_json and nlohmann::from_json follow the same design.
Meanwhile, we must rely on using unqualified calls:
// Pre C++20
template <typename T> void f(T const& container) {
using std::begin;
// If a free function begin exists in T's namespace, it will be picked up
// If not, std::begin will be used as a fallback
auto it = begin(container);
}
// C++20
template <typename T> void f(T const& container) {
// ranges::begin has the same behavior than the pre-C++20 mechanism.
auto it = ranges::begin(container);
}
Anyway, the using std::thing idiom is only necessary in generic code, and it's up to users to write it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Oh, I don't know why, but my brain made me believe you had put |
|
Ahh ok I understand! It’s all good, lord knows I would’ve actually put it in the code xD.
… On Apr 26, 2019, at 09:21, Théo DELRIEU ***@***.***> wrote:
Oh, I don't know why, but my brain made me believe you had put using std::to_string in the library's code... It's fine to let it in the tests :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@theodelrieu how do I use the ugly macro. I’m a bit confused on that front. Once I get that implemented I’ll close this and make a new PR |
|
No need to open a new PR, you can just rebase/squash your commits instead. Here is an example taken from the code: NLOHMANN_BASIC_JSON_TPL_DECLARATION
struct is_basic_json<NLOHMANN_BASIC_JSON_TPL> : std::true_type {};so basically you want: NLOHMANN_BASIC_JSON_TPL_DECLARATION
std::string to_string(const NLOHMANN_BASIC_JSON_TPL &j) { /* ... */ } |
|
@theodelrieu I updated and squashed and rebased. I apologize for the very last commit, rebasing caused some conflicts. But they are all resolved and ready to go! |
🔖 Release itemThis issue/PR will be part of the next release of the library. This template helps preparing the release notes. Type
Description
Contributor entry
|
I created the
to_stringfunction for thenlohmannnamespace and JSON. It uses the templateto easily accept any JSON or basic_json data type. This is in reference to Issue #916 to fully add
to_string.Note: As with all additives to the std namespace using ADL, it is required to declare
using std::to_stringotherwise the feature won't work.Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmanndirectory, runmake amalgamateto create the single-header filesingle_include/nlohmann/json.hpp. The whole process is described here.Please don't
#ifdefs or other means.