From 4a5ba093e7ab8eac7354180d749a322de60f7fa9 Mon Sep 17 00:00:00 2001
From: Eric Niebler <eniebler@fb.com>
Date: Fri, 12 Jan 2018 13:55:50 -0800
Subject: [PATCH] correctly handle APIs that accept Poly<T&> as an argument

Summary: Multi-dispatch in Poly was handled by treating arguments of type Poly<I&> as special, unwrapping them. That's a problem if the underlying API actually wants the Poly<I&> unmolested.

Reviewed By: yfeldblum

Differential Revision: D6713975

fbshipit-source-id: 18a90fa701fab14c3d3d46c247efe09ea5903b11
---
 folly/detail/PolyDetail.h | 42 ++++++++++++++++++++++++++++-----------
 folly/test/PolyTest.cpp   | 32 +++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/folly/detail/PolyDetail.h b/folly/detail/PolyDetail.h
index 144a9411..1a67b8e6 100644
--- a/folly/detail/PolyDetail.h
+++ b/folly/detail/PolyDetail.h
@@ -197,13 +197,23 @@ struct StaticConst {
 template <class T>
 constexpr T StaticConst<T>::value;
 
-template <class Fun>
-void if_constexpr(std::true_type, Fun fun) {
-  fun(Identity{});
+template <class Then>
+decltype(auto) if_constexpr(std::true_type, Then then) {
+  return then(Identity{});
 }
 
-template <class Fun>
-void if_constexpr(std::false_type, Fun) {}
+template <class Then>
+void if_constexpr(std::false_type, Then) {}
+
+template <class Then, class Else>
+decltype(auto) if_constexpr(std::true_type, Then then, Else) {
+  return then(Identity{});
+}
+
+template <class Then, class Else>
+decltype(auto) if_constexpr(std::false_type, Then, Else else_) {
+  return else_(Identity{});
+}
 
 enum class Op : short { eNuke, eMove, eCopy, eType, eAddr, eRefr };
 
@@ -468,14 +478,22 @@ T const& get(Data const& d) noexcept {
 
 enum class State : short { eEmpty, eInSitu, eOnHeap };
 
-template <class, class U>
-U&& convert(U&& u) noexcept {
-  return static_cast<U&&>(u);
-}
+template <class T>
+struct IsPolyRef : std::false_type {};
 
-template <class Arg, class I>
-decltype(auto) convert(Poly<I&> u) {
-  return poly_cast<Uncvref<Arg>>(u.get());
+template <class T>
+struct IsPolyRef<Poly<T&>> : std::true_type {};
+
+template <class Arg, class U>
+decltype(auto) convert(U&& u) {
+  return detail::if_constexpr(
+      StrictConjunction<
+          IsPolyRef<Uncvref<U>>,
+          Negation<std::is_convertible<U, Arg>>>(),
+      [&](auto id) -> decltype(auto) {
+        return poly_cast<Uncvref<Arg>>(id(u).get());
+      },
+      [&](auto id) -> U&& { return static_cast<U&&>(id(u)); });
 }
 
 template <class Fun>
diff --git a/folly/test/PolyTest.cpp b/folly/test/PolyTest.cpp
index 7083a464..7cb7e58b 100644
--- a/folly/test/PolyTest.cpp
+++ b/folly/test/PolyTest.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Facebook, Inc.
+ * Copyright 2017-present Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 5
 #pragma message "Folly.Poly requires gcc-5 or greater"
 #else
@@ -563,4 +562,33 @@ TEST(Poly, Addable) {
   cc = aref + bref;
   EXPECT_EQ(6, poly_cast<int>(cc));
 }
+
+namespace {
+struct IFrobnicator {
+  template <class Base>
+  struct Interface : Base {
+    void frobnicate(folly::Poly<folly::poly::IRegular&> x) {
+      folly::poly_call<0>(*this, x);
+    }
+  };
+  template <class T>
+  using Members = FOLLY_POLY_MEMBERS(&T::frobnicate);
+};
+using Frobnicator = folly::Poly<IFrobnicator>;
+
+struct my_frobnicator {
+  void frobnicate(folly::Poly<folly::poly::IRegular&>) {
+    // no-op
+  }
+};
+} // namespace
+
+TEST(Poly, PolyRefAsArg) {
+  folly::Poly<folly::poly::IRegular> x = 42;
+  Frobnicator frob = my_frobnicator{};
+  // should not throw:
+  frob.frobnicate(x);
+  // should not throw:
+  frob.frobnicate(folly::Poly<folly::poly::IRegular&>(x));
+}
 #endif
-- 
2.34.1