Commit c1a244da authored by Giuseppe Ottaviano's avatar Giuseppe Ottaviano Committed by JoelMarcey

Disable implicit conversions from std::string for non-char* Range

Summary:
`Range<Iter>` has an implicit constructors from strings for any
`Iter`, however such constructors are invalid (compilation fails)
if `Iter` is not `[const] char *`.

This can be an issue for overload resolution: for example

struct IsString {
bool operator()(folly::Range<int*>) { return false; }
bool operator()(folly::StringPiece) { return true; }
};

IsString()(std::string());

fails to compile because the overload is ambiguous, even if the
conversion to `ByteRange` is invalid.

This patch disables all the invalid constructors from
`[const] char*`, `std::string`, and `fbstring`.

Test Plan:
fbconfig -r folly && fbmake runtests_opt

Reviewed By: philipp@fb.com

Subscribers: folly-diffs@

FB internal diff: D1746899

Signature: t1:1746899:1418868361:50784c4993df0bd96eeb62c09c659d5e53964d9b
parent af581fbe
...@@ -121,6 +121,23 @@ value_before(Iter i) { ...@@ -121,6 +121,23 @@ value_before(Iter i) {
return *--i; return *--i;
} }
/*
* Use IsCharPointer<T>::type to enable const char* or char*.
* Use IsCharPointer<T>::const_type to enable only const char*.
*/
template <class T> struct IsCharPointer {};
template <>
struct IsCharPointer<char*> {
typedef int type;
};
template <>
struct IsCharPointer<const char*> {
typedef int const_type;
typedef int type;
};
} // namespace detail } // namespace detail
/** /**
...@@ -176,19 +193,19 @@ public: ...@@ -176,19 +193,19 @@ public:
: b_(start), e_(start + size) { } : b_(start), e_(start + size) { }
#if FOLLY_HAVE_CONSTEXPR_STRLEN #if FOLLY_HAVE_CONSTEXPR_STRLEN
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::type = 0>
constexpr /* implicit */ Range(Iter str) constexpr /* implicit */ Range(Iter str)
: b_(str), e_(str + strlen(str)) {} : b_(str), e_(str + strlen(str)) {}
#else #else
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::type = 0>
/* implicit */ Range(Iter str) /* implicit */ Range(Iter str)
: b_(str), e_(str + strlen(str)) {} : b_(str), e_(str + strlen(str)) {}
#endif #endif
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
/* implicit */ Range(const std::string& str) /* implicit */ Range(const std::string& str)
: b_(str.data()), e_(b_ + str.size()) {} : b_(str.data()), e_(b_ + str.size()) {}
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
Range(const std::string& str, std::string::size_type startFrom) { Range(const std::string& str, std::string::size_type startFrom) {
if (UNLIKELY(startFrom > str.size())) { if (UNLIKELY(startFrom > str.size())) {
throw std::out_of_range("index out of range"); throw std::out_of_range("index out of range");
...@@ -196,7 +213,7 @@ public: ...@@ -196,7 +213,7 @@ public:
b_ = str.data() + startFrom; b_ = str.data() + startFrom;
e_ = str.data() + str.size(); e_ = str.data() + str.size();
} }
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
Range(const std::string& str, Range(const std::string& str,
std::string::size_type startFrom, std::string::size_type startFrom,
std::string::size_type size) { std::string::size_type size) {
...@@ -210,6 +227,7 @@ public: ...@@ -210,6 +227,7 @@ public:
e_ = b_ + size; e_ = b_ + size;
} }
} }
template <class T = Iter, typename detail::IsCharPointer<T>::type = 0>
Range(const Range<Iter>& str, Range(const Range<Iter>& str,
size_t startFrom, size_t startFrom,
size_t size) { size_t size) {
...@@ -223,10 +241,10 @@ public: ...@@ -223,10 +241,10 @@ public:
e_ = b_ + size; e_ = b_ + size;
} }
} }
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
/* implicit */ Range(const fbstring& str) /* implicit */ Range(const fbstring& str)
: b_(str.data()), e_(b_ + str.size()) { } : b_(str.data()), e_(b_ + str.size()) { }
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
Range(const fbstring& str, fbstring::size_type startFrom) { Range(const fbstring& str, fbstring::size_type startFrom) {
if (UNLIKELY(startFrom > str.size())) { if (UNLIKELY(startFrom > str.size())) {
throw std::out_of_range("index out of range"); throw std::out_of_range("index out of range");
...@@ -234,7 +252,7 @@ public: ...@@ -234,7 +252,7 @@ public:
b_ = str.data() + startFrom; b_ = str.data() + startFrom;
e_ = str.data() + str.size(); e_ = str.data() + str.size();
} }
// Works only for Range<const char*> template <class T = Iter, typename detail::IsCharPointer<T>::const_type = 0>
Range(const fbstring& str, fbstring::size_type startFrom, Range(const fbstring& str, fbstring::size_type startFrom,
fbstring::size_type size) { fbstring::size_type size) {
if (UNLIKELY(startFrom > str.size())) { if (UNLIKELY(startFrom > str.size())) {
...@@ -358,10 +376,10 @@ public: ...@@ -358,10 +376,10 @@ public:
assert(b_ < e_); assert(b_ < e_);
return detail::value_before(e_); return detail::value_before(e_);
} }
// Works only for Range<const char*> // Works only for Range<const char*> and Range<char*>
std::string str() const { return std::string(b_, size()); } std::string str() const { return std::string(b_, size()); }
std::string toString() const { return str(); } std::string toString() const { return str(); }
// Works only for Range<const char*> // Works only for Range<const char*> and Range<char*>
fbstring fbstr() const { return fbstring(b_, size()); } fbstring fbstr() const { return fbstring(b_, size()); }
fbstring toFbstring() const { return fbstr(); } fbstring toFbstring() const { return fbstr(); }
...@@ -369,7 +387,7 @@ public: ...@@ -369,7 +387,7 @@ public:
return const_range_type(*this); return const_range_type(*this);
}; };
// Works only for Range<const char*> (and Range<char*>) // Works only for Range<const char*> and Range<char*>
int compare(const const_range_type& o) const { int compare(const const_range_type& o) const {
const size_type tsize = this->size(); const size_type tsize = this->size();
const size_type osize = o.size(); const size_type osize = o.size();
...@@ -399,7 +417,7 @@ public: ...@@ -399,7 +417,7 @@ public:
return b_[i]; return b_[i];
} }
// Works only for Range<const char*> // Works only for Range<const char*> and Range<char*>
uint32_t hash() const { uint32_t hash() const {
// Taken from fbi/nstring.h: // Taken from fbi/nstring.h:
// Quick and dirty bernstein hash...fine for short ascii strings // Quick and dirty bernstein hash...fine for short ascii strings
......
...@@ -805,6 +805,16 @@ TEST(StringPiece, split_step_with_process_range_delimiter_additional_args) { ...@@ -805,6 +805,16 @@ TEST(StringPiece, split_step_with_process_range_delimiter_additional_args) {
EXPECT_TRUE(p.empty()); EXPECT_TRUE(p.empty());
} }
TEST(StringPiece, NoInvalidImplicitConversions) {
struct IsString {
bool operator()(folly::Range<int*>) { return false; }
bool operator()(folly::StringPiece) { return true; }
};
std::string s = "hello";
EXPECT_TRUE(IsString()(s));
}
TEST(qfind, UInt32_Ranges) { TEST(qfind, UInt32_Ranges) {
vector<uint32_t> a({1, 2, 3, 260, 5}); vector<uint32_t> a({1, 2, 3, 260, 5});
vector<uint32_t> b({2, 3, 4}); vector<uint32_t> b({2, 3, 4});
......
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