Remove a use of SFINAE in the Range constructor.

Summary:
Specifically, the constructor `implicit Range(Iter)` is now declared and defined for all `Range` specializations. It is an error to use it, and we `static_assert`, unless `Iter` is `char const *` or `char *`.

Performance effect
 ---
Measuring compilation-time on a file that just make ~40k StringPieces, compiled with -O3. All compilers produced identical code before/after this change.

* clang-trunk: 4% improvement
* gcc-5: no change
* gcc-4.9: 11% improvement
* gcc-7.1: 5% improvement

Could this possibly break any existing code?
 ---
Yes. If you have a function that's overloaded for both `Range<char const*>` and `Range<X>` and your input object is not `char const*` and is implicitly convertible to both `char const*` and `X`: that is now ambiguous whereas before it would unambiguously pick the `Range<char const*>` overload. I don't consider this scenario likely.

Why should this work?
 ---
Using SFINAE is more expensive at compile time (with some compilation environments) than not using it. It's necessary to use SFINAE when there is an alternative function which will be used in preference when the substitution fails, but when that is not the case it is on average cheaper to make the function always exist and use static_assert to disallow the bad uses of it. A bonus is that the caller gets a more comprehensible error message.

Reviewed By: nbronson

Differential Revision: D5639502

fbshipit-source-id: 13469f2995a487398734f86108087fdc8e32ad71

diff --git a/folly/Range.h b/folly/Range.h
index 8ae96ce6cbe66fa2127e63824d38233ca02b4272..d1fa497c1eb9156c30144393bce0efd0bc309d40 100644 (file)
--- a/folly/Range.h
+++ b/folly/Range.h
@@ -212,9 +212,12 @@ class Range : private boost::totally_ordered<Range<Iter>> {
   /* implicit */ Range(std::nullptr_t) = delete;
 #endif
 
-  template <class T = Iter, typename detail::IsCharPointer<T>::type = 0>
   constexpr /* implicit */ Range(Iter str)
-      : b_(str), e_(str + constexpr_strlen(str)) {}
+      : b_(str), e_(str + constexpr_strlen(str)) {
+    static_assert(
+        std::is_same<int, typename detail::IsCharPointer<Iter>::type>::value,
+        "This constructor is only available for character ranges");
+  }
 
   template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
   /* implicit */ Range(const std::string& str)