-
Notifications
You must be signed in to change notification settings - Fork 0
Implement enum to string #89
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
base: main
Are you sure you want to change the base?
Conversation
|
Why is Windows build failing? P.S. |
|
I think the basic implementation is OK for a PoC quality; I'll be mentioning some functional issues in review. |
|
|
||
| template <class E> | ||
| requires std::is_enum_v<E> | ||
| constexpr std::string_view enum_to_string(E value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The basic name for this functionality should be
to_identifier, notto_string. We neednoexceptand[[nodiscard]]; we can treat this non-throwing version as the default (preferred) function. Since this version has the fallback value, it should be namedto_identifier_or. - In addition to the current version, we need another version that throws
class bad_enum_access : std::exceptionfor nonexistent values . Note that, even if the predefined enum members can be statically determined by reflection, there are still dynamic values for flag-like enums. This throwing version should be namedto_identifier. - Additionally, I think we can provide yet another version that delegates to
std::to_charsin case of nonexistent value. This version can be named as e.g.to_identifier_or_underlying.
| return std::meta::identifier_of(enumerator); | ||
| } | ||
| } | ||
| return "<unnamed>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be customizable via function parameter; something similar to std::optional::value_or is needed
| } // namespace yk | ||
|
|
||
| #endif | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some utility for adapting user-defined enums.
Such functionality should enable std::formatter and operator<<, and it should require minimal effort on the user's part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering yk::enum_serializer like std::formatter, defaults to use yk::enum_to_identifier. What do you think?
|
I think we can also provide a more rich version that resolves combination of flag-like values (e.g. |
|
Since we have this serialization feature now, I think we should also have the opposite one, i.e. enum parser. We should determine the priority for the parser implementation; I'm not sure if it should be included in the initial PR. |
| requires std::is_enum_v<E> | ||
| constexpr std::string_view enum_to_string(E value) | ||
| { | ||
| if constexpr (std::meta::is_enumerable_type(^^E)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this condition really needed? Do there exist such case that std::is_enum_v<E> == true && std::meta::is_enumerable_type(^^E) == false ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_enumerable_type returns false if E is incomplete type, which results to prevent enumerators_of being constant expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I think we need a test case for this.
closes #88
https://godbolt.org/z/Erq7bfevf