Commit e75cf21f authored by Ognjen Dragoljevic's avatar Ognjen Dragoljevic Committed by Facebook Github Bot

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
parent 968945c5
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include <folly/Exception.h> #include <folly/Exception.h>
#include <folly/FormatTraits.h> #include <folly/FormatTraits.h>
#include <folly/MapUtil.h>
#include <folly/Traits.h> #include <folly/Traits.h>
#include <folly/portability/Windows.h> #include <folly/portability/Windows.h>
...@@ -950,7 +951,10 @@ struct KeyableTraitsAssoc : public FormatTraitsBase { ...@@ -950,7 +951,10 @@ struct KeyableTraitsAssoc : public FormatTraitsBase {
typedef typename T::key_type key_type; typedef typename T::key_type key_type;
typedef typename T::value_type::second_type value_type; typedef typename T::value_type::second_type value_type;
static const value_type& at(const T& map, StringPiece key) { 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& static const value_type&
at(const T& map, StringPiece key, const value_type& dflt) { at(const T& map, StringPiece key, const value_type& dflt) {
......
...@@ -356,4 +356,15 @@ void insertThousandsGroupingUnsafe(char* start_buffer, char** end_buffer) { ...@@ -356,4 +356,15 @@ void insertThousandsGroupingUnsafe(char* start_buffer, char** end_buffer) {
} }
} // namespace detail } // 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 } // namespace folly
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#define FOLLY_FORMAT_H_ #define FOLLY_FORMAT_H_
#include <cstdio> #include <cstdio>
#include <stdexcept>
#include <tuple> #include <tuple>
#include <type_traits> #include <type_traits>
...@@ -306,6 +307,33 @@ inline std::string svformat(StringPiece fmt, Container&& container) { ...@@ -306,6 +307,33 @@ inline std::string svformat(StringPiece fmt, Container&& container) {
return vformat(fmt, std::forward<Container>(container)).str(); 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 * Wrap a sequence or associative container so that out-of-range lookups
* return a default value rather than throwing an exception. * return a default value rather than throwing an exception.
......
...@@ -162,6 +162,12 @@ TEST(Format, Simple) { ...@@ -162,6 +162,12 @@ TEST(Format, Simple) {
EXPECT_EQ("worldXX", svformat("{hello:X<7}", defaulted(m2, "meow"))); EXPECT_EQ("worldXX", svformat("{hello:X<7}", defaulted(m2, "meow")));
EXPECT_EQ("meowXXX", sformat("{[none]: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"))); 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 // Test indexing in strings
EXPECT_EQ("61 62", sformat("{0[0]:x} {0[1]:x}", "abcde")); EXPECT_EQ("61 62", sformat("{0[0]:x} {0[1]:x}", "abcde"));
......
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