From 96ac8f8ff0c9bc9446eede119fdfee0fe7feaba3 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Wed, 29 Nov 2017 22:13:39 -0800
Subject: [PATCH] Some fixes for custom conversions of enums

Summary:
[Folly] Some fixes for custom conversions of enums.

Of note, `to` was defined for enum -> all conversions, including enum -> string conversions, but we actually want enum -> string conversions to be done via ADL-discovered toAppend.

Reviewed By: ot

Differential Revision: D6411250

fbshipit-source-id: 852b64309e6adf1c68e5153635cb29632e2d86d4
---
 folly/Conv.h            | 11 +++++++----
 folly/test/ConvTest.cpp | 24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/folly/Conv.h b/folly/Conv.h
index 5a9e0069..a545816d 100644
--- a/folly/Conv.h
+++ b/folly/Conv.h
@@ -1546,7 +1546,8 @@ Tgt to(StringPiece* src) {
 
 template <class Tgt, class Src>
 typename std::enable_if<
-    std::is_enum<Src>::value && !std::is_same<Src, Tgt>::value,
+    std::is_enum<Src>::value && !std::is_same<Src, Tgt>::value &&
+        !std::is_convertible<Tgt, StringPiece>::value,
     Expected<Tgt, ConversionCode>>::type
 tryTo(const Src& value) {
   using I = typename std::underlying_type<Src>::type;
@@ -1555,7 +1556,8 @@ tryTo(const Src& value) {
 
 template <class Tgt, class Src>
 typename std::enable_if<
-    std::is_enum<Tgt>::value && !std::is_same<Src, Tgt>::value,
+    !std::is_convertible<Src, StringPiece>::valuea &&
+        std::is_enum<Tgt>::value && !std::is_same<Src, Tgt>::value,
     Expected<Tgt, ConversionCode>>::type
 tryTo(const Src& value) {
   using I = typename std::underlying_type<Tgt>::type;
@@ -1564,7 +1566,8 @@ tryTo(const Src& value) {
 
 template <class Tgt, class Src>
 typename std::enable_if<
-    std::is_enum<Src>::value && !std::is_same<Src, Tgt>::value,
+    std::is_enum<Src>::value && !std::is_same<Src, Tgt>::value &&
+        !std::is_convertible<Tgt, StringPiece>::value,
     Tgt>::type
 to(const Src& value) {
   return to<Tgt>(static_cast<typename std::underlying_type<Src>::type>(value));
@@ -1572,7 +1575,7 @@ to(const Src& value) {
 
 template <class Tgt, class Src>
 typename std::enable_if<
-    !IsSomeString<Src>::value && std::is_enum<Tgt>::value &&
+    !std::is_convertible<Src, StringPiece>::value && std::is_enum<Tgt>::value &&
         !std::is_same<Src, Tgt>::value,
     Tgt>::type
 to(const Src& value) {
diff --git a/folly/test/ConvTest.cpp b/folly/test/ConvTest.cpp
index 0f7b3669..8fe065b0 100644
--- a/folly/test/ConvTest.cpp
+++ b/folly/test/ConvTest.cpp
@@ -1239,6 +1239,22 @@ size_t estimateSpaceNeeded(const Dimensions&in) {
   return 2000 + folly::estimateSpaceNeeded(in.w) +
       folly::estimateSpaceNeeded(in.h);
 }
+
+enum class SmallEnum {};
+
+Expected<StringPiece, ConversionCode> parseTo(StringPiece in, SmallEnum& out) {
+  out = {};
+  if (in == "SmallEnum") {
+    return in.removePrefix(in), in;
+  } else {
+    return makeUnexpected(ConversionCode::STRING_TO_FLOAT_ERROR);
+  }
+}
+
+template <class String>
+void toAppend(SmallEnum, String* result) {
+  folly::toAppend("SmallEnum", result);
+}
 } // namespace my
 
 TEST(Conv, custom_kkproviders) {
@@ -1254,6 +1270,14 @@ TEST(Conv, custom_kkproviders) {
   EXPECT_EQ("7x8|7x8", str);
 }
 
+TEST(conv, custom_enumclass) {
+  EXPECT_EQ(my::SmallEnum{}, folly::to<my::SmallEnum>("SmallEnum"));
+  EXPECT_EQ(my::SmallEnum{}, folly::tryTo<my::SmallEnum>("SmallEnum").value());
+  auto str = to<string>(my::SmallEnum{});
+  toAppend("|", my::SmallEnum{}, &str);
+  EXPECT_EQ("SmallEnum|SmallEnum", str);
+}
+
 TEST(Conv, TryToThenWithVoid) {
   auto x = tryTo<int>("42").then([](int) {});
   EXPECT_TRUE(x.hasValue());
-- 
2.34.1