From cb4c59495e095a5d0e6f2d1ef8865c201610737a Mon Sep 17 00:00:00 2001
From: Victor Zverovich <victor.zverovich@gmail.com>
Date: Fri, 7 Jun 2019 07:08:04 -0700
Subject: [PATCH] Deprecate convert_to_int

---
 include/fmt/core.h    | 43 +++++++++++++++++++++++--------------------
 include/fmt/format.h  |  4 +---
 include/fmt/ostream.h |  8 --------
 test/core-test.cc     | 29 ++++++++++++++++++++++-------
 test/format-test.cc   |  4 ++--
 test/ostream-test.cc  | 26 ++++++++++++--------------
 6 files changed, 60 insertions(+), 54 deletions(-)

diff --git a/include/fmt/core.h b/include/fmt/core.h
index 0dea4e98..517eb387 100644
--- a/include/fmt/core.h
+++ b/include/fmt/core.h
@@ -483,17 +483,19 @@ template <typename Context> class basic_format_args;
 // A formatter for objects of type T.
 template <typename T, typename Char = char, typename Enable = void>
 struct formatter {
+  // A deleted default constructor indicates a disabled formatter.
   formatter() = delete;
 };
 
 template <typename T, typename Char, typename Enable = void>
-struct convert_to_int : bool_constant<!std::is_arithmetic<T>::value &&
-                                      std::is_convertible<T, int>::value> {};
+struct FMT_DEPRECATED convert_to_int
+    : bool_constant<!std::is_arithmetic<T>::value &&
+                    std::is_convertible<T, int>::value> {};
 
 namespace internal {
 
-// Specifies if T has an enabled formatter specialization. The type can be
-// formattable even if it doesn't have a formatter e.g. via conversion.
+// Specifies if T has an enabled formatter specialization. A type can be
+// formattable even if it doesn't have a formatter e.g. via a conversion.
 template <typename T, typename Context>
 using has_formatter =
     std::is_constructible<typename Context::template formatter_type<T>>;
@@ -603,16 +605,16 @@ inline Container& get_container(std::back_insert_iterator<Container> it) {
   return *accessor(it).container;
 }
 
-template <typename T> struct no_formatter_error : std::false_type {};
-
 template <typename T, typename Char = char, typename Enable = void>
 struct fallback_formatter {
-  static_assert(
-      no_formatter_error<T>::value,
-      "don't know how to format the type, include fmt/ostream.h if it provides "
-      "an operator<< that should be used");
+  fallback_formatter() = delete;
 };
 
+// Specifies if T has an enabled fallback_formatter specialization.
+template <typename T, typename Context>
+using has_fallback_formatter =
+    std::is_constructible<fallback_formatter<T, typename Context::char_type>>;
+
 template <typename Char> struct named_arg_base;
 
 template <typename T, typename Char> struct named_arg;
@@ -658,8 +660,6 @@ FMT_TYPE_CONSTANT(const Char*, cstring_type);
 FMT_TYPE_CONSTANT(basic_string_view<Char>, string_type);
 FMT_TYPE_CONSTANT(const void*, pointer_type);
 
-#undef FMT_TYPE_CONSTANT
-
 FMT_CONSTEXPR bool is_integral(type t) {
   FMT_ASSERT(t != internal::named_arg_type, "invalid argument type");
   return t > internal::none_type && t <= internal::last_integer_type;
@@ -795,6 +795,7 @@ FMT_MAKE_VALUE_SAME(long_long_type, long long)
 FMT_MAKE_VALUE_SAME(ulong_long_type, unsigned long long)
 FMT_MAKE_VALUE(int_type, signed char, int)
 FMT_MAKE_VALUE(uint_type, unsigned char, unsigned)
+FMT_MAKE_VALUE(char_type, char, int)
 
 // This doesn't use FMT_MAKE_VALUE because of ambiguity in gcc 4.4.
 template <typename C, typename Char,
@@ -803,10 +804,11 @@ FMT_CONSTEXPR init<C, int, char_type> make_value(Char val) {
   return {val};
 }
 
-template <typename C,
-          FMT_ENABLE_IF(!std::is_same<typename C::char_type, char>::value)>
-FMT_CONSTEXPR init<C, int, char_type> make_value(char val) {
-  return val;
+template <typename C, typename T,
+          FMT_ENABLE_IF(is_char<T>::value && !std::is_same<T, char>::value &&
+                        !std::is_same<T, typename C::char_type>::value)>
+FMT_CONSTEXPR init<C, int, char_type> make_value(const T&) {
+  static_assert(!sizeof(T), "mixing character types is disallowed");
 }
 
 FMT_MAKE_VALUE(double_type, float, double)
@@ -855,8 +857,8 @@ void make_value(const T*) {
 }
 
 template <typename C, typename T,
-          FMT_ENABLE_IF(std::is_enum<T>::value&&
-                            convert_to_int<T, typename C::char_type>::value)>
+          FMT_ENABLE_IF(std::is_enum<T>::value && !has_formatter<T, C>::value &&
+                        !has_fallback_formatter<T, C>::value)>
 inline init<C, int, int_type> make_value(const T& val) {
   return static_cast<int>(val);
 }
@@ -866,8 +868,9 @@ inline init<C, int, int_type> make_value(const T& val) {
 template <
     typename C, typename T, typename Char = typename C::char_type,
     typename U = typename std::remove_volatile<T>::type,
-    FMT_ENABLE_IF(!convert_to_int<U, Char>::value &&
-                  !std::is_same<U, Char>::value &&
+    FMT_ENABLE_IF(!std::is_same<U, Char>::value &&
+                  (!std::is_convertible<U, int>::value ||
+                   has_fallback_formatter<U, C>::value) &&
                   !std::is_convertible<U, basic_string_view<Char>>::value &&
                   !std::is_constructible<basic_string_view<Char>, U>::value &&
                   !internal::is_string<U>::value)>
diff --git a/include/fmt/format.h b/include/fmt/format.h
index fd12f99d..07b14e0c 100644
--- a/include/fmt/format.h
+++ b/include/fmt/format.h
@@ -3035,7 +3035,7 @@ class format_int {
   std::string str() const { return std::string(str_, size()); }
 };
 
-// formatter specializations for the core types corresponding to internal::type
+// A formatter specialization for the core types corresponding to internal::type
 // constants.
 template <typename T, typename Char>
 struct formatter<T, Char,
@@ -3131,8 +3131,6 @@ FMT_FORMAT_AS(Char*, const Char*);
 FMT_FORMAT_AS(std::basic_string<Char>, basic_string_view<Char>);
 FMT_FORMAT_AS(std::nullptr_t, const void*);
 
-#undef FMT_FORMAT_AS
-
 template <typename Char>
 struct formatter<void*, Char> : formatter<const void*, Char> {
   template <typename FormatContext>
diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h
index e1b7e2b6..49aa5805 100644
--- a/include/fmt/ostream.h
+++ b/include/fmt/ostream.h
@@ -109,14 +109,6 @@ struct fallback_formatter<T, Char,
 };
 }  // namespace internal
 
-// Disable conversion to int if T has an overloaded operator<< which is a free
-// function (not a member of std::ostream).
-template <typename T, typename Char>
-struct convert_to_int<T, Char,
-                      enable_if_t<internal::is_streamable<T, Char>::value>> {
-  static const bool value = false;
-};
-
 template <typename Char>
 void vprint(std::basic_ostream<Char>& os, basic_string_view<Char> format_str,
             basic_format_args<buffer_context<Char>> args) {
diff --git a/test/core-test.cc b/test/core-test.cc
index 549a87c8..474c4973 100644
--- a/test/core-test.cc
+++ b/test/core-test.cc
@@ -432,14 +432,29 @@ TEST(StringViewTest, Compare) {
   check_op<std::greater_equal>();
 }
 
-enum basic_enum {};
-enum enum_with_underlying_type : char {};
+struct enabled_formatter {};
+struct disabled_formatter {};
+struct disabled_formatter_convertible {
+  operator int() const { return 42; }
+};
 
-TEST(CoreTest, ConvertToInt) {
-  EXPECT_FALSE((fmt::convert_to_int<char, char>::value));
-  EXPECT_FALSE((fmt::convert_to_int<const char*, char>::value));
-  EXPECT_TRUE((fmt::convert_to_int<basic_enum, char>::value));
-  EXPECT_TRUE((fmt::convert_to_int<enum_with_underlying_type, char>::value));
+FMT_BEGIN_NAMESPACE
+template <> struct formatter<enabled_formatter> {
+  auto parse(format_parse_context& ctx) -> decltype(ctx.begin()) {
+    return ctx.begin();
+  }
+  auto format(enabled_formatter, format_context& ctx) -> decltype(ctx.out()) {
+    return ctx.out();
+  }
+};
+FMT_END_NAMESPACE
+
+TEST(CoreTest, HasFormatter) {
+  using fmt::internal::has_formatter;
+  using context = fmt::format_context;
+  EXPECT_TRUE((has_formatter<enabled_formatter, context>::value));
+  EXPECT_FALSE((has_formatter<disabled_formatter, context>::value));
+  EXPECT_FALSE((has_formatter<disabled_formatter_convertible, context>::value));
 }
 
 struct convertible_to_int {
diff --git a/test/format-test.cc b/test/format-test.cc
index d98c7791..0157b65c 100644
--- a/test/format-test.cc
+++ b/test/format-test.cc
@@ -1877,8 +1877,8 @@ enum TestEnum { A };
 TEST(FormatTest, Enum) { EXPECT_EQ("0", fmt::format("{}", A)); }
 
 TEST(FormatTest, FormatterNotSpecialized) {
-  EXPECT_FALSE((fmt::internal::has_formatter<
-    fmt::formatter<TestEnum>, fmt::format_context>::value));
+  EXPECT_FALSE((fmt::internal::has_formatter<fmt::formatter<TestEnum>,
+                                             fmt::format_context>::value));
 }
 
 #if FMT_HAS_FEATURE(cxx_strong_enums)
diff --git a/test/ostream-test.cc b/test/ostream-test.cc
index 3527958b..27c62347 100644
--- a/test/ostream-test.cc
+++ b/test/ostream-test.cc
@@ -46,24 +46,22 @@ struct type_with_comma_op {};
 template <typename T> void operator,(type_with_comma_op, const T&);
 template <typename T> type_with_comma_op operator<<(T&, const Date&);
 
-enum TestEnum {};
-static std::ostream& operator<<(std::ostream& os, TestEnum) {
-  return os << "TestEnum";
+enum streamable_enum {};
+static std::ostream& operator<<(std::ostream& os, streamable_enum) {
+  return os << "streamable_enum";
 }
 
-static std::wostream& operator<<(std::wostream& os, TestEnum) {
-  return os << L"TestEnum";
+static std::wostream& operator<<(std::wostream& os, streamable_enum) {
+  return os << L"streamable_enum";
 }
 
-enum TestEnum2 { A };
+enum unstreamable_enum {};
 
 TEST(OStreamTest, Enum) {
-  EXPECT_FALSE((fmt::convert_to_int<TestEnum, char>::value));
-  EXPECT_EQ("TestEnum", fmt::format("{}", TestEnum()));
-  EXPECT_EQ("0", fmt::format("{}", A));
-  EXPECT_FALSE((fmt::convert_to_int<TestEnum, wchar_t>::value));
-  EXPECT_EQ(L"TestEnum", fmt::format(L"{}", TestEnum()));
-  EXPECT_EQ(L"0", fmt::format(L"{}", A));
+  EXPECT_EQ("streamable_enum", fmt::format("{}", streamable_enum()));
+  EXPECT_EQ("0", fmt::format("{}", unstreamable_enum()));
+  EXPECT_EQ(L"streamable_enum", fmt::format(L"{}", streamable_enum()));
+  EXPECT_EQ(L"0", fmt::format(L"{}", unstreamable_enum()));
 }
 
 typedef fmt::back_insert_range<fmt::internal::buffer<char>> range;
@@ -81,8 +79,8 @@ TEST(OStreamTest, CustomArg) {
   fmt::format_specs spec;
   test_arg_formatter af(ctx, spec);
   fmt::visit_format_arg(
-      af, fmt::internal::make_arg<fmt::format_context>(TestEnum()));
-  EXPECT_EQ("TestEnum", std::string(buffer.data(), buffer.size()));
+      af, fmt::internal::make_arg<fmt::format_context>(streamable_enum()));
+  EXPECT_EQ("streamable_enum", std::string(buffer.data(), buffer.size()));
 }
 
 TEST(OStreamTest, Format) {