Commit 6078246a authored by Phil Willoughby's avatar Phil Willoughby Committed by Facebook Github Bot

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
parent 1eb414a3
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment