Commit 11505903 authored by Phil Willoughby's avatar Phil Willoughby Committed by Facebook Github Bot

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
parent 9773e441
...@@ -159,11 +159,7 @@ BaseFormatter<Derived, containerMode, Args...>::BaseFormatter( ...@@ -159,11 +159,7 @@ BaseFormatter<Derived, containerMode, Args...>::BaseFormatter(
Args&&... args) Args&&... args)
: str_(str), : str_(str),
values_(FormatValue<typename std::decay<Args>::type>( values_(FormatValue<typename std::decay<Args>::type>(
std::forward<Args>(args))...) { std::forward<Args>(args))...) {}
static_assert(
!containerMode || sizeof...(Args) == 1,
"Exactly one argument required in container mode");
}
template <class Derived, bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
template <class Output> template <class Output>
...@@ -246,7 +242,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()( ...@@ -246,7 +242,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
arg.widthIndex == FormatArg::kNoIndex, arg.widthIndex == FormatArg::kNoIndex,
"cannot provide width arg index without value arg index"); "cannot provide width arg index without value arg index");
int sizeArg = nextArg++; int sizeArg = nextArg++;
arg.width = getSizeArg(size_t(sizeArg), arg); arg.width = asDerived().getSizeArg(size_t(sizeArg), arg);
} }
argIndex = nextArg++; argIndex = nextArg++;
...@@ -256,7 +252,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()( ...@@ -256,7 +252,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
arg.enforce( arg.enforce(
arg.widthIndex != FormatArg::kNoIndex, arg.widthIndex != FormatArg::kNoIndex,
"cannot provide value arg index without width arg index"); "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 { try {
...@@ -274,7 +270,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()( ...@@ -274,7 +270,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
"folly::format: may not have both default and explicit arg indexes"); "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);
} }
} }
......
...@@ -56,9 +56,14 @@ class FormatterTag {}; ...@@ -56,9 +56,14 @@ class FormatterTag {};
* this directly, you have to use format(...) below. * this directly, you have to use format(...) below.
*/ */
/* BaseFormatter class. Currently, the only behavior that can be /* BaseFormatter class.
* overridden is the actual formatting of positional parameters in * Overridable behaviours:
* You may override the actual formatting of positional parameters in
* `doFormatArg`. The Formatter class provides the default implementation. * `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> template <class Derived, bool containerMode, class... Args>
class BaseFormatter { class BaseFormatter {
...@@ -108,6 +113,10 @@ class BaseFormatter { ...@@ -108,6 +113,10 @@ class BaseFormatter {
ValueTuple; ValueTuple;
static constexpr size_t valueCount = std::tuple_size<ValueTuple>::value; 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> template <size_t K, class Callback>
typename std::enable_if<K == valueCount>::type typename std::enable_if<K == valueCount>::type
doFormatFrom(size_t i, FormatArg& arg, Callback& /*cb*/) const { doFormatFrom(size_t i, FormatArg& arg, Callback& /*cb*/) const {
...@@ -118,7 +127,7 @@ class BaseFormatter { ...@@ -118,7 +127,7 @@ class BaseFormatter {
typename std::enable_if<(K < valueCount)>::type typename std::enable_if<(K < valueCount)>::type
doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const { doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
if (i == K) { if (i == K) {
static_cast<const Derived*>(this)->template doFormatArg<K>(arg, cb); asDerived().template doFormatArg<K>(arg, cb);
} else { } else {
doFormatFrom<K + 1>(i, arg, cb); doFormatFrom<K + 1>(i, arg, cb);
} }
...@@ -196,7 +205,11 @@ class Formatter : public BaseFormatter< ...@@ -196,7 +205,11 @@ class Formatter : public BaseFormatter<
: BaseFormatter< : BaseFormatter<
Formatter<containerMode, Args...>, Formatter<containerMode, Args...>,
containerMode, 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> template <size_t K, class Callback>
void doFormatArg(FormatArg& arg, Callback& cb) const { void doFormatArg(FormatArg& arg, Callback& cb) const {
......
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