From: Ognjen Dragoljevic <plamenko@fb.com>
Date: Wed, 1 Nov 2017 22:54:38 +0000 (-0700)
Subject: Make associative container out-of-range exception provide missing key
X-Git-Tag: v2017.11.06.00~15
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=e75cf21f157eee116bd55cbffa68de02831ed775

Make associative container out-of-range exception provide missing key

Summary:
When the key is missing, the standard associative containers throw an exception like `unordered_map::at: key not found`. That's nice, but it would be even better if we would actually know what key is missing. This is not an issue when one is accessing the container directly, but when that access happens several levels deep, such as here within folly formatting logic, we have no way of knowing what key was missing. This poses some difficulties in presenting error to the user.
This change makes folly format throw a subclass of `std::out_of_range` exception on a missing key when a string keyed associative container is used for providing parameters. That subclass stores the actual key used so it can be accessed in the exception handler. Existing callers can still catch `std::out_of_range` so they should be unaffected by this change.

Reviewed By: ot, yfeldblum

Differential Revision: D6202184

fbshipit-source-id: b8a6740aaccc5d8914ad7d099c8b901427f00083
---

diff --git a/folly/Format-inl.h b/folly/Format-inl.h
index 60fdf245..b97375c1 100644
--- a/folly/Format-inl.h
+++ b/folly/Format-inl.h
@@ -27,6 +27,7 @@
 
 #include <folly/Exception.h>
 #include <folly/FormatTraits.h>
+#include <folly/MapUtil.h>
 #include <folly/Traits.h>
 #include <folly/portability/Windows.h>
 
@@ -950,7 +951,10 @@ struct KeyableTraitsAssoc : public FormatTraitsBase {
   typedef typename T::key_type key_type;
   typedef typename T::value_type::second_type value_type;
   static const value_type& at(const T& map, StringPiece key) {
-    return map.at(KeyFromStringPiece<key_type>::convert(key));
+    if (auto ptr = get_ptr(map, KeyFromStringPiece<key_type>::convert(key))) {
+      return *ptr;
+    }
+    detail::throwFormatKeyNotFoundException(key);
   }
   static const value_type&
   at(const T& map, StringPiece key, const value_type& dflt) {
diff --git a/folly/Format.cpp b/folly/Format.cpp
index 380cddbe..9ad5395e 100644
--- a/folly/Format.cpp
+++ b/folly/Format.cpp
@@ -356,4 +356,15 @@ void insertThousandsGroupingUnsafe(char* start_buffer, char** end_buffer) {
 }
 } // namespace detail
 
+FormatKeyNotFoundException::FormatKeyNotFoundException(StringPiece key)
+    : std::out_of_range(kMessagePrefix.str() + key.str()) {}
+
+constexpr StringPiece const FormatKeyNotFoundException::kMessagePrefix;
+
+namespace detail {
+[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key) {
+  throw FormatKeyNotFoundException(key);
+}
+} // namespace detail
+
 } // namespace folly
diff --git a/folly/Format.h b/folly/Format.h
index 3837d107..b8df86fb 100644
--- a/folly/Format.h
+++ b/folly/Format.h
@@ -18,6 +18,7 @@
 #define FOLLY_FORMAT_H_
 
 #include <cstdio>
+#include <stdexcept>
 #include <tuple>
 #include <type_traits>
 
@@ -306,6 +307,33 @@ inline std::string svformat(StringPiece fmt, Container&& container) {
   return vformat(fmt, std::forward<Container>(container)).str();
 }
 
+/**
+ * Exception class thrown when a format key is not found in the given
+ * associative container keyed by strings. We inherit std::out_of_range for
+ * compatibility with callers that expect exception to be thrown directly
+ * by std::map or std::unordered_map.
+ *
+ * Having the key be at the end of the message string, we can access it by
+ * simply adding its offset to what(). Not storing separate std::string key
+ * makes the exception type small and noexcept-copyable like std::out_of_range,
+ * and therefore able to fit in-situ in exception_wrapper.
+ */
+class FormatKeyNotFoundException : public std::out_of_range {
+ public:
+  explicit FormatKeyNotFoundException(StringPiece key);
+
+  char const* key() const noexcept {
+    return what() + kMessagePrefix.size();
+  }
+
+ private:
+  static constexpr StringPiece const kMessagePrefix = "format key not found: ";
+};
+
+namespace detail {
+[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key);
+} // namespace detail
+
 /**
  * Wrap a sequence or associative container so that out-of-range lookups
  * return a default value rather than throwing an exception.
diff --git a/folly/test/FormatTest.cpp b/folly/test/FormatTest.cpp
index 3bb739bb..e84bc862 100644
--- a/folly/test/FormatTest.cpp
+++ b/folly/test/FormatTest.cpp
@@ -162,6 +162,12 @@ TEST(Format, Simple) {
   EXPECT_EQ("worldXX", svformat("{hello:X<7}", defaulted(m2, "meow")));
   EXPECT_EQ("meowXXX", sformat("{[none]:X<7}", defaulted(m2, "meow")));
   EXPECT_EQ("meowXXX", svformat("{none:X<7}", defaulted(m2, "meow")));
+  try {
+    svformat("{none:X<7}", m2);
+    EXPECT_FALSE(true) << "svformat should throw on missing key";
+  } catch (const FormatKeyNotFoundException& e) {
+    EXPECT_STREQ("none", e.key());
+  }
 
   // Test indexing in strings
   EXPECT_EQ("61 62", sformat("{0[0]:x} {0[1]:x}", "abcde"));