revert format()'s behavior of crashing on error

Summary:
This reverts 61e20daa, which changed the format code to abort on error.

I do plan to restore the crashing behavior, but I plan to make it
optional.  I will add a formatSafe() function that throws on error,
while format() catches these exceptions and crashes.

This diff is an intermediate diff to make it easier to review the
changes.  This is close to a straight revert of 61e20daa and 61a41c9b.
However, I did leave the new test case, as well as the
FormatArg::errorStr() method added by those diffs.

Test Plan:
Verified that the existing format tests pass.

Also added a handful of new tests for bad format strings.  This did
catch one bug in the code where it wasn't correctly checking for a null
return value from memchr().

Reviewed By: delong.j@fb.com

FB internal diff: D1144298

diff --git a/folly/Format-inl.h b/folly/Format-inl.h
index 8f4bb8e95a080ff74f60103f235e861aa24de7d8..f33ff62b274f1f36a44de0e21da2a7fc2f97f54e 100644 (file)
--- a/folly/Format-inl.h
+++ b/folly/Format-inl.h
@@ -169,7 +169,10 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
       out(StringPiece(p, q));
       p = q;
 
       out(StringPiece(p, q));
       p = q;
 

-      CHECK(p != end && *p == '}') << "single '}' in format string";
+      if (p == end || *p != '}') {
+        throw std::invalid_argument(
+            "folly::format: single '}' in format string");
+      }

       ++p;
     }
   };
       ++p;
     }
   };


@@ -186,7 +189,10 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
     outputString(StringPiece(p, q));
     p = q + 1;
 
     outputString(StringPiece(p, q));
     p = q + 1;
 

-    CHECK(p != end) << "'{' at end of format string";
+    if (p == end) {
+      throw std::invalid_argument(
+          "folly::format: '}' at end of format string");
+    }

 
     // "{{" -> "{"
     if (*p == '{') {
 
     // "{{" -> "{"
     if (*p == '{') {


@@ -197,7 +203,9 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
 
     // Format string
     q = static_cast<const char*>(memchr(p, '}', end - p));
 
     // Format string
     q = static_cast<const char*>(memchr(p, '}', end - p));

-    CHECK(q != end) << "missing ending '}'";
+    if (q == nullptr) {
+      throw std::invalid_argument("folly::format: missing ending '}'");
+    }

     FormatArg arg(StringPiece(p, q));
     p = q + 1;
 
     FormatArg arg(StringPiece(p, q));
     p = q + 1;
 


@@ -219,16 +227,17 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
         try {
           argIndex = to<int>(piece);
         } catch (const std::out_of_range& e) {
         try {
           argIndex = to<int>(piece);
         } catch (const std::out_of_range& e) {

-          LOG(FATAL) << "argument index must be integer";
+          arg.error("argument index must be integer");

         }
         }

-        CHECK(argIndex >= 0)
-          << arg.errorStr("argument index must be non-negative");
+        arg.enforce(argIndex >= 0, "argument index must be non-negative");

         hasExplicitArgIndex = true;
       }
     }
 
         hasExplicitArgIndex = true;
       }
     }
 

-    CHECK(!hasDefaultArgIndex || !hasExplicitArgIndex)
-      << "may not have both default and explicit arg indexes";
+    if (hasDefaultArgIndex && hasExplicitArgIndex) {
+      throw std::invalid_argument(
+          "folly::format: may not have both default and explicit arg indexes");
+    }

 
     doFormat(argIndex, arg, out);
   }
 
     doFormat(argIndex, arg, out);
   }


@@ -403,8 +412,8 @@ class FormatValue<
       uval = val_;
       sign = '\0';
 
       uval = val_;
       sign = '\0';
 

-      CHECK(arg.sign == FormatArg::Sign::DEFAULT)
-        << arg.errorStr("sign specifications not allowed for unsigned values");
+      arg.enforce(arg.sign == FormatArg::Sign::DEFAULT,
+                  "sign specifications not allowed for unsigned values");

     }
 
     // max of:
     }
 
     // max of:


@@ -431,9 +440,9 @@ class FormatValue<
     switch (presentation) {
     case 'n':  // TODO(tudorb): locale awareness?
     case 'd':
     switch (presentation) {
     case 'n':  // TODO(tudorb): locale awareness?
     case 'd':

-      CHECK(!arg.basePrefix)
-        << arg.errorStr("base prefix not allowed with '", presentation,
-                        "' specifier");
+      arg.enforce(!arg.basePrefix,
+                  "base prefix not allowed with '", presentation,
+                  "' specifier");

       if (arg.thousandsSeparator) {
         useSprintf("%'ju");
       } else {
       if (arg.thousandsSeparator) {
         useSprintf("%'ju");
       } else {


@@ -443,21 +452,21 @@ class FormatValue<
       }
       break;
     case 'c':
       }
       break;
     case 'c':

-      CHECK(!arg.basePrefix)
-        << arg.errorStr("base prefix not allowed with '", presentation,
-                        "' specifier");
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.basePrefix,
+                  "base prefix not allowed with '", presentation,
+                  "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");

       valBufBegin = valBuf + 3;
       *valBufBegin = static_cast<char>(uval);
       valBufEnd = valBufBegin + 1;
       break;
     case 'o':
     case 'O':
       valBufBegin = valBuf + 3;
       *valBufBegin = static_cast<char>(uval);
       valBufEnd = valBufBegin + 1;
       break;
     case 'o':
     case 'O':

-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");

       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToOctal(valBuf, valBufSize - 1, uval);
       if (arg.basePrefix) {
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToOctal(valBuf, valBufSize - 1, uval);
       if (arg.basePrefix) {


@@ -466,9 +475,9 @@ class FormatValue<
       }
       break;
     case 'x':
       }
       break;
     case 'x':

-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");

       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexLower(valBuf, valBufSize - 1,
                                                     uval);
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexLower(valBuf, valBufSize - 1,
                                                     uval);


@@ -479,9 +488,9 @@ class FormatValue<
       }
       break;
     case 'X':
       }
       break;
     case 'X':

-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");

       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexUpper(valBuf, valBufSize - 1,
                                                     uval);
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexUpper(valBuf, valBufSize - 1,
                                                     uval);


@@ -493,9 +502,9 @@ class FormatValue<
       break;
     case 'b':
     case 'B':
       break;
     case 'b':
     case 'B':

-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");

       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToBinary(valBuf, valBufSize - 1,
                                                   uval);
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToBinary(valBuf, valBufSize - 1,
                                                   uval);


@@ -506,7 +515,7 @@ class FormatValue<
       }
       break;
     default:
       }
       break;
     default:

-      LOG(FATAL) << arg.errorStr("invalid specifier '", presentation, "'");
+      arg.error("invalid specifier '", presentation, "'");

     }
 
     if (sign) {
     }
 
     if (sign) {


@@ -624,7 +633,7 @@ class FormatValue<double> {
             exponentSymbol,
             -4, arg.precision,
             0, 0);
             exponentSymbol,
             -4, arg.precision,
             0, 0);

-        CHECK(conv.ToExponential(val, arg.precision, &builder));
+        arg.enforce(conv.ToExponential(val, arg.precision, &builder));

       }
       break;
     case 'n':  // should be locale-aware, but isn't
       }
       break;
     case 'n':  // should be locale-aware, but isn't


@@ -644,11 +653,11 @@ class FormatValue<double> {
             exponentSymbol,
             -4, arg.precision,
             0, 0);
             exponentSymbol,
             -4, arg.precision,
             0, 0);

-        CHECK(conv.ToShortest(val, &builder));
+        arg.enforce(conv.ToShortest(val, &builder));

       }
       break;
     default:
       }
       break;
     default:

-      LOG(FATAL) << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.error("invalid specifier '", arg.presentation, "'");

     }
 
     int len = builder.position();
     }
 
     int len = builder.position();


@@ -705,9 +714,9 @@ class FormatValue<
   void format(FormatArg& arg, FormatCallback& cb) const {
     if (arg.keyEmpty()) {
       arg.validate(FormatArg::Type::OTHER);
   void format(FormatArg& arg, FormatCallback& cb) const {
     if (arg.keyEmpty()) {
       arg.validate(FormatArg::Type::OTHER);

-      CHECK(arg.presentation == FormatArg::kDefaultPresentation ||
-            arg.presentation == 's')
-        << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.enforce(arg.presentation == FormatArg::kDefaultPresentation ||
+                  arg.presentation == 's',
+                  "invalid specifier '", arg.presentation, "'");

       format_value::formatString(val_, arg, cb);
     } else {
       FormatValue<char>(val_.at(arg.splitIntKey())).format(arg, cb);
       format_value::formatString(val_, arg, cb);
     } else {
       FormatValue<char>(val_.at(arg.splitIntKey())).format(arg, cb);


@@ -727,8 +736,8 @@ class FormatValue<std::nullptr_t> {
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     arg.validate(FormatArg::Type::OTHER);
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     arg.validate(FormatArg::Type::OTHER);

-    CHECK(arg.presentation == FormatArg::kDefaultPresentation)
-      << arg.errorStr("invalid specifier '", arg.presentation, "'");
+    arg.enforce(arg.presentation == FormatArg::kDefaultPresentation,
+                "invalid specifier '", arg.presentation, "'");

     format_value::formatString("(null)", arg, cb);
   }
 };
     format_value::formatString("(null)", arg, cb);
   }
 };


@@ -778,8 +787,8 @@ class FormatValue<
     } else {
       // Print as a pointer, in hex.
       arg.validate(FormatArg::Type::OTHER);
     } else {
       // Print as a pointer, in hex.
       arg.validate(FormatArg::Type::OTHER);

-      CHECK(arg.presentation == FormatArg::kDefaultPresentation)
-        << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.enforce(arg.presentation == FormatArg::kDefaultPresentation,
+                  "invalid specifier '", arg.presentation, "'");

       arg.basePrefix = true;
       arg.presentation = 'x';
       if (arg.align == FormatArg::Align::DEFAULT) {
       arg.basePrefix = true;
       arg.presentation = 'x';
       if (arg.align == FormatArg::Align::DEFAULT) {


@@ -799,7 +808,7 @@ class TryFormatValue {
  public:
   template <class FormatCallback>
   static void formatOrFail(T& value, FormatArg& arg, FormatCallback& cb) {
  public:
   template <class FormatCallback>
   static void formatOrFail(T& value, FormatArg& arg, FormatCallback& cb) {

-    LOG(FATAL) << arg.errorStr("No formatter available for this type");
+    arg.error("No formatter available for this type");

   }
 };
 
   }
 };
 


@@ -1040,7 +1049,7 @@ class FormatValue<std::pair<A, B>> {
       FormatValue<typename std::decay<B>::type>(val_.second).format(arg, cb);
       break;
     default:
       FormatValue<typename std::decay<B>::type>(val_.second).format(arg, cb);
       break;
     default:

-      LOG(FATAL) << arg.errorStr("invalid index for pair");
+      arg.error("invalid index for pair");

     }
   }
 
     }
   }
 


@@ -1058,7 +1067,7 @@ class FormatValue<std::tuple<Args...>> {
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     int key = arg.splitIntKey();
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     int key = arg.splitIntKey();

-    CHECK(key >= 0) << arg.errorStr("tuple index must be non-negative");
+    arg.enforce(key >= 0, "tuple index must be non-negative");

     doFormat(key, arg, cb);
   }
 
     doFormat(key, arg, cb);
   }
 


@@ -1068,7 +1077,7 @@ class FormatValue<std::tuple<Args...>> {
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {

-    LOG(FATAL) << arg.errorStr("tuple index out of range, max=", i);
+    arg.enforce("tuple index out of range, max=", i);

   }
 
   template <size_t K, class Callback>
   }
 
   template <size_t K, class Callback>

diff --git a/folly/Format.cpp b/folly/Format.cpp
index 169eca087192066ddc1963c547727a81b70052d7..553c550abd46d5c56449dedeec6d757db4a27867 100644 (file)
--- a/folly/Format.cpp
+++ b/folly/Format.cpp
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -70,7 +70,7 @@ void FormatArg::initSlow() {
     }
 
     if (*p == '0') {
     }
 
     if (*p == '0') {

-      CHECK(align == Align::DEFAULT) << errorStr("alignment specified twice");
+      enforce(align == Align::DEFAULT, "alignment specified twice");

       fill = '0';
       align = Align::PAD_AFTER_SIGN;
       if (++p == end) return;
       fill = '0';
       align = Align::PAD_AFTER_SIGN;
       if (++p == end) return;


@@ -105,31 +105,31 @@ void FormatArg::initSlow() {
     if (++p == end) return;
   }
 
     if (++p == end) return;
   }
 

-  LOG(FATAL) << "extra characters in format string";
+  error("extra characters in format string");

 }
 
 void FormatArg::validate(Type type) const {
 }
 
 void FormatArg::validate(Type type) const {

-  CHECK(keyEmpty()) << "index not allowed";
+  enforce(keyEmpty(), "index not allowed");

   switch (type) {
   case Type::INTEGER:
   switch (type) {
   case Type::INTEGER:

-    CHECK(precision == kDefaultPrecision)
-      << errorStr("precision not allowed on integers");
+    enforce(precision == kDefaultPrecision,
+            "precision not allowed on integers");

     break;
   case Type::FLOAT:
     break;
   case Type::FLOAT:

-    CHECK(!basePrefix)
-      << errorStr("base prefix ('#') specifier only allowed on integers");
-    CHECK(!thousandsSeparator)
-      << errorStr("thousands separator (',') only allowed on integers");
+    enforce(!basePrefix,
+            "base prefix ('#') specifier only allowed on integers");
+    enforce(!thousandsSeparator,
+            "thousands separator (',') only allowed on integers");

     break;
   case Type::OTHER:
     break;
   case Type::OTHER:

-    CHECK(align != Align::PAD_AFTER_SIGN)
-      << errorStr("'='alignment only allowed on numbers");
-    CHECK(sign == Sign::DEFAULT)
-      << errorStr("sign specifier only allowed on numbers");
-    CHECK(!basePrefix)
-      << errorStr("base prefix ('#') specifier only allowed on integers");
-    CHECK(!thousandsSeparator)
-      << errorStr("thousands separator (',') only allowed on integers");
+    enforce(align != Align::PAD_AFTER_SIGN,
+            "'='alignment only allowed on numbers");
+    enforce(sign == Sign::DEFAULT,
+            "sign specifier only allowed on numbers");
+    enforce(!basePrefix,
+            "base prefix ('#') specifier only allowed on integers");
+    enforce(!thousandsSeparator,
+            "thousands separator (',') only allowed on integers");

     break;
   }
 }
     break;
   }
 }

diff --git a/folly/Format.h b/folly/Format.h
index 589bcdff4b59d3f18f3100a299b56010a5a72fa6..da9b83f82bce46387881f8b5c5542b7674bf6c8c 100644 (file)
--- a/folly/Format.h
+++ b/folly/Format.h
@@ -122,7 +122,7 @@ class Formatter {
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {

-    LOG(FATAL) << arg.errorStr("argument index out of range, max=", i);
+    arg.error("argument index out of range, max=", i);

   }
 
   template <size_t K, class Callback>
   }
 
   template <size_t K, class Callback>

diff --git a/folly/FormatArg.h b/folly/FormatArg.h
index d48aad7a05b277f5b3c1866d7399a49f71b83e8a..4099c13005f2227a78bb425ca140e7a0c150ac29 100644 (file)
--- a/folly/FormatArg.h
+++ b/folly/FormatArg.h
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -202,7 +202,7 @@ inline void FormatArg::error(Args&&... args) const {
 
 template <bool emptyOk>
 inline StringPiece FormatArg::splitKey() {
 
 template <bool emptyOk>
 inline StringPiece FormatArg::splitKey() {

-  CHECK(nextKeyMode_ != NextKeyMode::INT) << errorStr("integer key expected");
+  enforce(nextKeyMode_ != NextKeyMode::INT, "integer key expected");

   return doSplitKey<emptyOk>();
 }
 
   return doSplitKey<emptyOk>();
 }
 


@@ -210,12 +210,16 @@ template <bool emptyOk>
 inline StringPiece FormatArg::doSplitKey() {
   if (nextKeyMode_ == NextKeyMode::STRING) {
     nextKeyMode_ = NextKeyMode::NONE;
 inline StringPiece FormatArg::doSplitKey() {
   if (nextKeyMode_ == NextKeyMode::STRING) {
     nextKeyMode_ = NextKeyMode::NONE;

-    CHECK(emptyOk || !nextKey_.empty()) << errorStr("non-empty key required");
+    if (!emptyOk) {  // static
+      enforce(!nextKey_.empty(), "non-empty key required");
+    }

     return nextKey_;
   }
 
   if (key_.empty()) {
     return nextKey_;
   }
 
   if (key_.empty()) {

-    CHECK(emptyOk) << errorStr("non-empty key required");
+    if (!emptyOk) {  // static
+      error("non-empty key required");
+    }

     return StringPiece();
   }
 
     return StringPiece();
   }
 


@@ -225,7 +229,7 @@ inline StringPiece FormatArg::doSplitKey() {
   if (e[-1] == ']') {
     --e;
     p = static_cast<const char*>(memchr(b, '[', e - b));
   if (e[-1] == ']') {
     --e;
     p = static_cast<const char*>(memchr(b, '[', e - b));

-    CHECK(p) << errorStr("unmatched ']'");
+    enforce(p, "unmatched ']'");

   } else {
     p = static_cast<const char*>(memchr(b, '.', e - b));
   }
   } else {
     p = static_cast<const char*>(memchr(b, '.', e - b));
   }


@@ -235,8 +239,9 @@ inline StringPiece FormatArg::doSplitKey() {
     p = e;
     key_.clear();
   }
     p = e;
     key_.clear();
   }

-  CHECK(emptyOk || b != p) << errorStr("non-empty key required");
-
+  if (!emptyOk) {  // static
+    enforce(b != p, "non-empty key required");
+  }

   return StringPiece(b, p);
 }
 
   return StringPiece(b, p);
 }
 


@@ -248,7 +253,7 @@ inline int FormatArg::splitIntKey() {
   try {
     return to<int>(doSplitKey<true>());
   } catch (const std::out_of_range& e) {
   try {
     return to<int>(doSplitKey<true>());
   } catch (const std::out_of_range& e) {

-    LOG(FATAL) << errorStr("integer key required");
+    error("integer key required");

     return 0;  // unreached
   }
 }
     return 0;  // unreached
   }
 }

diff --git a/folly/test/FormatTest.cpp b/folly/test/FormatTest.cpp
index e81266ea7ccd244dd777bac8e63620c18a4e1709..49a0794976dc946618181be4dc540b513a3f20b1 100644 (file)
--- a/folly/test/FormatTest.cpp
+++ b/folly/test/FormatTest.cpp
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -296,6 +296,20 @@ TEST(Format, Custom) {
   EXPECT_NE("", fstr("{}", &kv));
 }
 
   EXPECT_NE("", fstr("{}", &kv));
 }
 

+namespace {
+
+struct Opaque {
+  int k;
+};
+
+} // namespace
+
+TEST(Format, Unformatted) {
+  Opaque o;
+  EXPECT_NE("", fstr("{}", &o));
+  EXPECT_THROW(fstr("{0[0]}", &o), std::invalid_argument);
+}
+

 TEST(Format, Nested) {
   EXPECT_EQ("1 2 3 4", fstr("{} {} {}", 1, 2, format("{} {}", 3, 4)));
   //
 TEST(Format, Nested) {
   EXPECT_EQ("1 2 3 4", fstr("{} {} {}", 1, 2, format("{} {}", 3, 4)));
   //


@@ -312,6 +326,18 @@ TEST(Format, OutOfBounds) {
   std::map<std::string, int> map{{"hello", 0}, {"world", 1}};
   EXPECT_EQ("hello = 0", fstr("hello = {[hello]}", map));
   EXPECT_THROW(fstr("{[nope]}", map), std::out_of_range);
   std::map<std::string, int> map{{"hello", 0}, {"world", 1}};
   EXPECT_EQ("hello = 0", fstr("hello = {[hello]}", map));
   EXPECT_THROW(fstr("{[nope]}", map), std::out_of_range);

+  EXPECT_THROW(vstr("{nope}", map), std::out_of_range);
+}
+
+TEST(Format, BogusFormatString) {
+  EXPECT_THROW(fstr("}"), std::invalid_argument);
+  EXPECT_THROW(fstr("foo}bar"), std::invalid_argument);
+  EXPECT_THROW(fstr("foo{bar"), std::invalid_argument);
+  EXPECT_THROW(fstr("{[test]"), std::invalid_argument);
+
+  // This one fails in detail::enforceWhitespace(), which throws
+  // std::range_error
+  EXPECT_THROW(fstr("{0[test}"), std::exception);

 }
 
 int main(int argc, char *argv[]) {
 }
 
 int main(int argc, char *argv[]) {