From 115059030653fe6f73e351073097ba1e06507f21 Mon Sep 17 00:00:00 2001
From: Phil Willoughby <philwill@fb.com>
Date: Tue, 7 Feb 2017 00:45:01 -0800
Subject: [PATCH] Modify BaseFormatter to help static analysers

Summary:
This decouples the format-string-parser from the storage of the arguments to be
formatted: a static analyser can now define a class which derives from
`BaseFormatter` with no storage and use `BaseFormatter::operator()` to verify
the syntax of the format string and determine which arguments will be
referenced.

This method of allowing overrides is resolved at compile time; the benchmarks
confirm that there is no run-time difference.

Reviewed By: yfeldblum

Differential Revision: D4507689

fbshipit-source-id: f109d81ae54dc074ac363720ef6e565520435d26
---
 folly/Format-inl.h | 12 ++++--------
 folly/Format.h     | 21 +++++++++++++++++----
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/folly/Format-inl.h b/folly/Format-inl.h
index 1b998dec..70f5564d 100644
--- a/folly/Format-inl.h
+++ b/folly/Format-inl.h
@@ -159,11 +159,7 @@ BaseFormatter<Derived, containerMode, Args...>::BaseFormatter(
     Args&&... args)
     : str_(str),
       values_(FormatValue<typename std::decay<Args>::type>(
-          std::forward<Args>(args))...) {
-  static_assert(
-      !containerMode || sizeof...(Args) == 1,
-      "Exactly one argument required in container mode");
-}
+          std::forward<Args>(args))...) {}
 
 template <class Derived, bool containerMode, class... Args>
 template <class Output>
@@ -246,7 +242,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
               arg.widthIndex == FormatArg::kNoIndex,
               "cannot provide width arg index without value arg index");
           int sizeArg = nextArg++;
-          arg.width = getSizeArg(size_t(sizeArg), arg);
+          arg.width = asDerived().getSizeArg(size_t(sizeArg), arg);
         }
 
         argIndex = nextArg++;
@@ -256,7 +252,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
           arg.enforce(
               arg.widthIndex != FormatArg::kNoIndex,
               "cannot provide value arg index without width arg index");
-          arg.width = getSizeArg(size_t(arg.widthIndex), arg);
+          arg.width = asDerived().getSizeArg(size_t(arg.widthIndex), arg);
         }
 
         try {
@@ -274,7 +270,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
           "folly::format: may not have both default and explicit arg indexes");
     }
 
-    doFormat(size_t(argIndex), arg, out);
+    asDerived().template doFormat(size_t(argIndex), arg, out);
   }
 }
 
diff --git a/folly/Format.h b/folly/Format.h
index f87d90a9..8986c4ca 100644
--- a/folly/Format.h
+++ b/folly/Format.h
@@ -56,9 +56,14 @@ class FormatterTag {};
  * this directly, you have to use format(...) below.
  */
 
-/* BaseFormatter class. Currently, the only behavior that can be
- * overridden is the actual formatting of positional parameters in
+/* BaseFormatter class.
+ * Overridable behaviours:
+ * You may override the actual formatting of positional parameters in
  * `doFormatArg`. The Formatter class provides the default implementation.
+ *
+ * You may also override `doFormat` and `getSizeArg`. These override points were
+ * added to permit static analysis of format strings, when it is inconvenient
+ * or impossible to instantiate a BaseFormatter with the correct storage
  */
 template <class Derived, bool containerMode, class... Args>
 class BaseFormatter {
@@ -108,6 +113,10 @@ class BaseFormatter {
       ValueTuple;
   static constexpr size_t valueCount = std::tuple_size<ValueTuple>::value;
 
+  Derived const& asDerived() const {
+    return *static_cast<const Derived*>(this);
+  }
+
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& /*cb*/) const {
@@ -118,7 +127,7 @@ class BaseFormatter {
   typename std::enable_if<(K < valueCount)>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
     if (i == K) {
-      static_cast<const Derived*>(this)->template doFormatArg<K>(arg, cb);
+      asDerived().template doFormatArg<K>(arg, cb);
     } else {
       doFormatFrom<K + 1>(i, arg, cb);
     }
@@ -196,7 +205,11 @@ class Formatter : public BaseFormatter<
       : BaseFormatter<
             Formatter<containerMode, Args...>,
             containerMode,
-            Args...>(str, std::forward<Args>(args)...) {}
+            Args...>(str, std::forward<Args>(args)...) {
+    static_assert(
+        !containerMode || sizeof...(Args) == 1,
+        "Exactly one argument required in container mode");
+  }
 
   template <size_t K, class Callback>
   void doFormatArg(FormatArg& arg, Callback& cb) const {
-- 
2.34.1