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

Upgrading to 3.x: to_/from_json with enum class #1093

Closed
endorph-soft opened this issue May 17, 2018 · 13 comments
Closed

Upgrading to 3.x: to_/from_json with enum class #1093

endorph-soft opened this issue May 17, 2018 · 13 comments
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@endorph-soft
Copy link

endorph-soft commented May 17, 2018

I'm in the process of updating a project from version 2.1.1 to the version 3.x series.

I have the following two functions defined to handle enum classes:

template <typename JSON, typename Enum>
void to_json(JSON & j, Enum e) {
  j = ToString(e);
}

template <typename JSON, typename Enum>
void from_json(const JSON & j, Enum & e) {
  if (!FromString(e, j)) {
    throw std::invalid_argument("Badness...");
  }
}

There are definitions of ToString and FromString for all the enumerations of interest. I'm not using SFINAE to restrict Enum to enumerations, because it's not currently needed. With version 2.1.1, everything is working fine.

With the update to 3.x, these functions are no longer found by the lookup. If I comment out the new enum to_json and from_json inside the library, the errors disappear, and all is well again.

How can I make my functions work? Do I need a more specific signature? I don't want to define a function for every enumeration, if I can get away with it.

@nlohmann
Copy link
Owner

Are the functions in the same namespace as the respective Enum?

@endorph-soft
Copy link
Author

Yep, although they are in a different header file (which is included). The internal ToString and FromString are in the global namespace, but I don't think that matters.

It all works fine with 2.1.1.

@theodelrieu
Copy link
Contributor

I'll try to take a look in the next days, meanwhile I would suggest partially specialize adl_serializer on enum classes.

I don't remember when we did add support for enums, I think it was in 2.1.0, which would be even stranger that it breaks now

@endorph-soft
Copy link
Author

In 2.1.1 there is code for Unscoped Enums, using this:

template<typename T>
using is_unscoped_enum = 
    std::integral_constant<bool, std::is_convertible<T, int>::value and 
    std::is_enum<T>::value>;

I don't think it will pick up enum classes (by design, probably). The newer code picks up both enum and enum class.

I'll see if I can figure out the adl_serialiser thing. Thanks.

@theodelrieu
Copy link
Contributor

I did take a look, and in order to pick up your overload, you have to constrain your template a bit:

template <typename JSON, typename Enum, typename = std::enable_if_t<std::is_enum<Enum>::value>>
void to_json(JSON & j, Enum e) {
  j = ToString(e);
}

No need to fiddle with adl_serializer in this case.

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label May 23, 2018
@endorph-soft
Copy link
Author

Thanks for that, and apologies for the slow response. It'll be a few days before I can get back to rolling out new library versions, but I'm sure it will work. Thanks.

@nlohmann
Copy link
Owner

Any news on this?

@endorph-soft
Copy link
Author

Sorry for the lack of response. We've been approaching a release, and I've been busy putting out other fires. I'll try and have a look shortly. Thanks again for your help, and for the library. It's very useful.

@nlohmann
Copy link
Owner

@endorph-soft Any news one this?

@nlohmann
Copy link
Owner

nlohmann commented Jul 9, 2018

Can I close this issue?

@alexhadd
Copy link

alexhadd commented Jul 10, 2018

Hi everyone, I was solving a problem very similar to this, and @theodelrieu 's response was very close to being correct. I could not get things to work until I changed the template a little. I went from...

template <typename JSON, typename Enum, typename = std::enable_if_t<std::is_enum<Enum>::value>>
void to_json(JSON & j, Enum e) {
 j = ToString(e);
}

to

typename Enum, typename = std::enable_if_t<std::is_enum<Enum>::value>>
void to_json(nlohmann::json& j, Enum e) {
  j = ToString(e);
}

Before making this change, I was still being told that my type had no from_json defined when using .get<namespace::mytype>(). I'm quite uncertain as to why directly referring to the nlohmann::json object fixed the issue, but it did.

@nlohmann
Copy link
Owner

Thanks for checking back!

@theodelrieu
Copy link
Contributor

Hmm, this is quite weird, I'd like to know what is the real difference between both...

Anyway, glad you could solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

4 participants