Replace CHECK in Range.h by throw std::out_of_range

Summary:
Calling CHECK() in folly will force the program to abort in case of a failure.
On the other hand, for range checking, the standard library throws std::out_of_range for many functions.

Thus it could be a good idea to throw the same exception in folly so that errors can be handled using try {} catch (...) {} blocks.

Test Plan:
from fbcode, type:
fbconfig -r folly; fbmake opt -j32; fbmake runtests_opt -j 32

What other tests should I run?

Reviewed By: tudorb@fb.com

FB internal diff: D808204

diff --git a/folly/Range.cpp b/folly/Range.cpp
index c1e58eda0120c4b4b6d50cbf73a29da86ed820c7..b4359bb1e56c0d84c04f0bd6f639d857a6c53c0f 100644 (file)
--- a/folly/Range.cpp
+++ b/folly/Range.cpp
@@ -21,7 +21,6 @@
 
 #include <emmintrin.h>  // __v16qi
 #include <iostream>
-#include "folly/Likely.h"
 
 namespace folly {
 

diff --git a/folly/Range.h b/folly/Range.h
index 43bebb2ba51bff9af5e349316663b242e1f084b0..04ea64ef74147f6584b7189ac1cdb3e3c97b213d 100644 (file)
--- a/folly/Range.h
+++ b/folly/Range.h
@@ -33,6 +33,7 @@
 #include <bits/c++config.h>
 #include "folly/CpuId.h"
 #include "folly/Traits.h"
+#include "folly/Likely.h"
 
 namespace folly {
 
@@ -143,7 +144,9 @@ public:
       : b_(str.data()), e_(b_ + str.size()) {}
   // Works only for Range<const char*>
   Range(const std::string& str, std::string::size_type startFrom) {
-    CHECK_LE(startFrom, str.size());
+    if (UNLIKELY(startFrom > str.size())) {
+      throw std::out_of_range("index out of range");
+    }
     b_ = str.data() + startFrom;
     e_ = str.data() + str.size();
   }
@@ -151,32 +154,52 @@ public:
   Range(const std::string& str,
         std::string::size_type startFrom,
         std::string::size_type size) {
-    CHECK_LE(startFrom + size, str.size());
+    if (UNLIKELY(startFrom > str.size())) {
+      throw std::out_of_range("index out of range");
+    }
     b_ = str.data() + startFrom;
-    e_ = b_ + size;
+    if (str.size() - startFrom < size) {
+      e_ = str.data() + str.size();
+    } else {
+      e_ = b_ + size;
+    }
   }
   Range(const Range<Iter>& str,
         size_t startFrom,
         size_t size) {
-    CHECK_LE(startFrom + size, str.size());
+    if (UNLIKELY(startFrom > str.size())) {
+      throw std::out_of_range("index out of range");
+    }
     b_ = str.b_ + startFrom;
-    e_ = b_ + size;
+    if (str.size() - startFrom < size) {
+      e_ = str.e_;
+    } else {
+      e_ = b_ + size;
+    }
   }
   // Works only for Range<const char*>
   /* implicit */ Range(const fbstring& str)
     : b_(str.data()), e_(b_ + str.size()) { }
   // Works only for Range<const char*>
   Range(const fbstring& str, fbstring::size_type startFrom) {
-    CHECK_LE(startFrom, str.size());
+    if (UNLIKELY(startFrom > str.size())) {
+      throw std::out_of_range("index out of range");
+    }
     b_ = str.data() + startFrom;
     e_ = str.data() + str.size();
   }
   // Works only for Range<const char*>
   Range(const fbstring& str, fbstring::size_type startFrom,
         fbstring::size_type size) {
-    CHECK_LE(startFrom + size, str.size());
+    if (UNLIKELY(startFrom > str.size())) {
+      throw std::out_of_range("index out of range");
+    }
     b_ = str.data() + startFrom;
-    e_ = b_ + size;
+    if (str.size() - startFrom < size) {
+      e_ = str.data() + str.size();
+    } else {
+      e_ = b_ + size;
+    }
   }
 
   // Allow implicit conversion from Range<const char*> (aka StringPiece) to
@@ -299,12 +322,16 @@ public:
   }
 
   void advance(size_type n) {
-    CHECK_LE(n, size());
+    if (n > size()) {
+      throw std::out_of_range("index out of range");
+    }
     b_ += n;
   }
 
   void subtract(size_type n) {
-    CHECK_LE(n, size());
+    if (n > size()) {
+      throw std::out_of_range("index out of range");
+    }
     e_ -= n;
   }
 
@@ -320,7 +347,9 @@ public:
 
   Range subpiece(size_type first,
                  size_type length = std::string::npos) const {
-    CHECK_LE(first, size());
+    if (first > size()) {
+      throw std::out_of_range("index out of range");
+    }
     return Range(b_ + first,
                  std::min<std::string::size_type>(length, size() - first));
   }

diff --git a/folly/test/RangeTest.cpp b/folly/test/RangeTest.cpp
index 095cb3a5a63511fb550a6b7907d61370fbb7819e..ebc20a5f221a5ff6c8add654eed5d5e2337507de 100644 (file)
--- a/folly/test/RangeTest.cpp
+++ b/folly/test/RangeTest.cpp
@@ -221,6 +221,24 @@ TEST(StringPiece, ToByteRange) {
   EXPECT_EQ(a.end(), c.end());
 }
 
+TEST(StringPiece, InvalidRange) {
+  StringPiece a("hello");
+  EXPECT_EQ(a, a.subpiece(0, 10));
+  EXPECT_EQ(StringPiece("ello"), a.subpiece(1));
+  EXPECT_EQ(StringPiece("ello"), a.subpiece(1, std::string::npos));
+  EXPECT_EQ(StringPiece("ell"), a.subpiece(1, 3));
+  EXPECT_THROW(a.subpiece(6, 7), std::out_of_range);
+  EXPECT_THROW(a.subpiece(6), std::out_of_range);
+
+  std::string b("hello");
+  EXPECT_EQ(a, StringPiece(b, 0, 10));
+  EXPECT_EQ("ello", a.subpiece(1));
+  EXPECT_EQ("ello", a.subpiece(1, std::string::npos));
+  EXPECT_EQ("ell", a.subpiece(1, 3));
+  EXPECT_THROW(a.subpiece(6, 7), std::out_of_range);
+  EXPECT_THROW(a.subpiece(6), std::out_of_range);
+}
+
 template <typename NeedleFinder>
 class NeedleFinderTest : public ::testing::Test {
  public: