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

Map not int enum to correct underlying_type #1286

Merged
merged 2 commits into from
Aug 31, 2019
Merged

Conversation

agmt
Copy link
Contributor

@agmt agmt commented Aug 29, 2019

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

This PR correctly prints values of such enums:
enum E{ I = 5000000000ULL };

Test update is needed because "0" may not detect an error.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@@ -1906,9 +1906,9 @@ TEST(FormatTest, FormatterNotSpecialized) {
}

#if FMT_HAS_FEATURE(cxx_strong_enums)
enum TestFixedEnum : short { B };
enum TestFixedEnum : short { B = 1 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it actually formatted differently if we don't map to underlying type?

Copy link
Contributor Author

@agmt agmt Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the problem clearly but if map() returns short (which doesn't enumerated in enum type)

FMT_ENABLE_IF(std::is_enum<T>::value &&
                          !has_formatter<T, Context>::value &&
                          !has_fallback_formatter<T, Context>::value)>
  FMT_CONSTEXPR typename std::underlying_type<T>::type map(const T& val) {
    return static_cast<typename std::underlying_type<T>::type>(val);
  }

then after that will be called map(short) with correct argument and map(int) with garbage ( =0 almost always). It means that fmt::format(short_enum_value) will return "0" regardless of the argument.

@vitaut vitaut merged commit bcd9b93 into fmtlib:master Aug 31, 2019
@vitaut
Copy link
Contributor

vitaut commented Aug 31, 2019

Anyway, I merged the PR and will add a regression test separately. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants