Commit 664e121a authored by Tyler MacDonald's avatar Tyler MacDonald Committed by Tudor Bosman

make `folly::Formatter` extendible

Summary:
on advice of @tudorb, move most of `folly::Formatter` into `folly::BaseFormatter` so that we can use compile-time polymorphism to provide different types of Formatters.

I wasn't able to get the recursive formatter to be polymorphic -- whenever I tried to convert `class FormatValue<Formatter<containerMode, Args...>, void>` into `class FormatValue<BaseFormatter...`, `FormatTest.cpp:Test(Format, Nested)` wouldn't compile because it couldn't find the template. @tudorb, if you have an easy fix for this, lmk, otherwise I'm (reluctantly) okay with requiring that `Formatter`s define their own nesting `FormatValue`.

phew. the last time I did this sort of metaprogramming was over 5 years ago in perl. Doing it in C++ is... interesting.

Test Plan: `fbconfig -r thrift folly cold_storage && fbmake dbg && fbmake runtests`

Reviewed By: tudorb@fb.com

Subscribers: tudorb, dgp

FB internal diff: D1422343

Tasks: 4624268
parent eaed5551
...@@ -141,18 +141,19 @@ size_t uintToBinary(char* buffer, size_t bufLen, Uint v) { ...@@ -141,18 +141,19 @@ size_t uintToBinary(char* buffer, size_t bufLen, Uint v) {
} // namespace detail } // namespace detail
template <class Derived, bool containerMode, class... Args>
template <bool containerMode, class... Args> BaseFormatter<Derived, containerMode, Args...>::BaseFormatter(StringPiece str,
Formatter<containerMode, Args...>::Formatter(StringPiece str, 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, static_assert(!containerMode || sizeof...(Args) == 1,
"Exactly one argument required in container mode"); "Exactly one argument required in container mode");
} }
template <bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
void Formatter<containerMode, Args...>::handleFormatStrError() const { void BaseFormatter<Derived, containerMode, Args...>::handleFormatStrError()
const {
if (crashOnError_) { if (crashOnError_) {
LOG(FATAL) << "folly::format: bad format string \"" << str_ << "\": " << LOG(FATAL) << "folly::format: bad format string \"" << str_ << "\": " <<
folly::exceptionStr(std::current_exception()); folly::exceptionStr(std::current_exception());
...@@ -160,9 +161,10 @@ void Formatter<containerMode, Args...>::handleFormatStrError() const { ...@@ -160,9 +161,10 @@ void Formatter<containerMode, Args...>::handleFormatStrError() const {
throw; throw;
} }
template <bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
template <class Output> template <class Output>
void Formatter<containerMode, Args...>::operator()(Output& out) const { void BaseFormatter<Derived, containerMode, Args...>::operator()(Output& out)
const {
// Catch BadFormatArg and range_error exceptions, and call // Catch BadFormatArg and range_error exceptions, and call
// handleFormatStrError(). // handleFormatStrError().
// //
...@@ -193,9 +195,10 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const { ...@@ -193,9 +195,10 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
} }
} }
template <bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
template <class Output> template <class Output>
void Formatter<containerMode, Args...>::appendOutput(Output& out) const { void BaseFormatter<Derived, containerMode, Args...>::appendOutput(Output& out)
const {
auto p = str_.begin(); auto p = str_.begin();
auto end = str_.end(); auto end = str_.end();
...@@ -287,8 +290,9 @@ void Formatter<containerMode, Args...>::appendOutput(Output& out) const { ...@@ -287,8 +290,9 @@ void Formatter<containerMode, Args...>::appendOutput(Output& out) const {
} }
} }
template <bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
void writeTo(FILE* fp, const Formatter<containerMode, Args...>& formatter) { void writeTo(FILE* fp,
const BaseFormatter<Derived, containerMode, Args...>& formatter) {
auto writer = [fp] (StringPiece sp) { auto writer = [fp] (StringPiece sp) {
ssize_t n = fwrite(sp.data(), 1, sp.size(), fp); ssize_t n = fwrite(sp.data(), 1, sp.size(), fp);
if (n < sp.size()) { if (n < sp.size()) {
...@@ -367,10 +371,14 @@ void formatNumber(StringPiece val, int prefixLen, FormatArg& arg, ...@@ -367,10 +371,14 @@ void formatNumber(StringPiece val, int prefixLen, FormatArg& arg,
format_value::formatString(val, arg, cb); format_value::formatString(val, arg, cb);
} }
template <class FormatCallback, bool containerMode, class... Args> template <class FormatCallback,
void formatFormatter(const Formatter<containerMode, Args...>& formatter, class Derived,
FormatArg& arg, bool containerMode,
FormatCallback& cb) { class... Args>
void formatFormatter(
const BaseFormatter<Derived, containerMode, Args...>& formatter,
FormatArg& arg,
FormatCallback& cb) {
if (arg.width == FormatArg::kDefaultWidth && if (arg.width == FormatArg::kDefaultWidth &&
arg.precision == FormatArg::kDefaultPrecision) { arg.precision == FormatArg::kDefaultPrecision) {
// nothing to do // nothing to do
...@@ -1204,9 +1212,14 @@ class FormatValue<std::tuple<Args...>> { ...@@ -1204,9 +1212,14 @@ class FormatValue<std::tuple<Args...>> {
}; };
// Partial specialization of FormatValue for nested Formatters // Partial specialization of FormatValue for nested Formatters
template <bool containerMode, class... Args> template <bool containerMode,
class FormatValue<Formatter<containerMode, Args...>, void> { class... Args,
typedef Formatter<containerMode, Args...> FormatterValue; template <bool containerMode, class... Args> class F>
class FormatValue<F<containerMode, Args...>,
typename std::enable_if<detail::IsFormatter<
F<containerMode, Args...>>::value>::type> {
typedef typename F<containerMode, Args...>::BaseType FormatterValue;
public: public:
explicit FormatValue(const FormatterValue& f) : f_(f) { } explicit FormatValue(const FormatterValue& f) : f_(f) { }
...@@ -1222,10 +1235,9 @@ class FormatValue<Formatter<containerMode, Args...>, void> { ...@@ -1222,10 +1235,9 @@ class FormatValue<Formatter<containerMode, Args...>, void> {
* Formatter objects can be appended to strings, and therefore they're * Formatter objects can be appended to strings, and therefore they're
* compatible with folly::toAppend and folly::to. * compatible with folly::toAppend and folly::to.
*/ */
template <class Tgt, bool containerMode, class... Args> template <class Tgt, class Derived, bool containerMode, class... Args>
typename std::enable_if< typename std::enable_if<IsSomeString<Tgt>::value>::type toAppend(
IsSomeString<Tgt>::value>::type const BaseFormatter<Derived, containerMode, Args...>& value, Tgt* result) {
toAppend(const Formatter<containerMode, Args...>& value, Tgt * result) {
value.appendTo(*result); value.appendTo(*result);
} }
......
...@@ -51,6 +51,11 @@ template <class C> ...@@ -51,6 +51,11 @@ template <class C>
Formatter<true, C> vformat(StringPiece fmt, C&& container); Formatter<true, C> vformat(StringPiece fmt, C&& container);
template <class T, class Enable=void> class FormatValue; template <class T, class Enable=void> class FormatValue;
// meta-attribute to identify formatters in this sea of template weirdness
namespace detail {
class FormatterTag {};
};
/** /**
* Formatter class. * Formatter class.
* *
...@@ -59,8 +64,12 @@ template <class T, class Enable=void> class FormatValue; ...@@ -59,8 +64,12 @@ template <class T, class Enable=void> class FormatValue;
* this directly, you have to use format(...) below. * this directly, you have to use format(...) below.
*/ */
template <bool containerMode, class... Args> /* BaseFormatter class. Currently, the only behavior that can be
class Formatter { * overridden is the actual formatting of positional parameters in
* `doFormatArg`. The Formatter class provides the default implementation.
*/
template <class Derived, bool containerMode, class... Args>
class BaseFormatter {
public: public:
/* /*
* Change whether or not Formatter should crash or throw exceptions if the * Change whether or not Formatter should crash or throw exceptions if the
...@@ -109,21 +118,13 @@ class Formatter { ...@@ -109,21 +118,13 @@ class Formatter {
return s; return s;
} }
private: /**
explicit Formatter(StringPiece str, Args&&... args); * metadata to identify generated children of BaseFormatter
*/
// Not copyable typedef detail::FormatterTag IsFormatter;
Formatter(const Formatter&) = delete; typedef BaseFormatter BaseType;
Formatter& operator=(const Formatter&) = delete;
// Movable, but the move constructor and assignment operator are private,
// for the exclusive use of format() (below). This way, you can't create
// a Formatter object, but can handle references to it (for streaming,
// conversion to string, etc) -- which is good, as Formatter objects are
// dangerous (they hold references, possibly to temporaries)
Formatter(Formatter&&) = default;
Formatter& operator=(Formatter&&) = default;
private:
typedef std::tuple<FormatValue< typedef std::tuple<FormatValue<
typename std::decay<Args>::type>...> ValueTuple; typename std::decay<Args>::type>...> ValueTuple;
static constexpr size_t valueCount = std::tuple_size<ValueTuple>::value; static constexpr size_t valueCount = std::tuple_size<ValueTuple>::value;
...@@ -142,7 +143,7 @@ class Formatter { ...@@ -142,7 +143,7 @@ class Formatter {
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) {
std::get<K>(values_).format(arg, cb); static_cast<const Derived*>(this)->template doFormatArg<K>(arg, cb);
} else { } else {
doFormatFrom<K+1>(i, arg, cb); doFormatFrom<K+1>(i, arg, cb);
} }
...@@ -154,9 +155,45 @@ class Formatter { ...@@ -154,9 +155,45 @@ class Formatter {
} }
StringPiece str_; StringPiece str_;
ValueTuple values_;
bool crashOnError_{true}; bool crashOnError_{true};
protected:
explicit BaseFormatter(StringPiece str, Args&&... args);
// Not copyable
BaseFormatter(const BaseFormatter&) = delete;
BaseFormatter& operator=(const BaseFormatter&) = delete;
// Movable, but the move constructor and assignment operator are private,
// for the exclusive use of format() (below). This way, you can't create
// a Formatter object, but can handle references to it (for streaming,
// conversion to string, etc) -- which is good, as Formatter objects are
// dangerous (they hold references, possibly to temporaries)
BaseFormatter(BaseFormatter&&) = default;
BaseFormatter& operator=(BaseFormatter&&) = default;
ValueTuple values_;
};
template <bool containerMode, class... Args>
class Formatter : public BaseFormatter<Formatter<containerMode, Args...>,
containerMode,
Args...> {
private:
explicit Formatter(StringPiece& str, Args&&... args)
: BaseFormatter<Formatter<containerMode, Args...>,
containerMode,
Args...>(str, std::forward<Args>(args)...) {}
template <size_t K, class Callback>
void doFormatArg(FormatArg& arg, Callback& cb) const {
std::get<K>(this->values_).format(arg, cb);
}
friend class BaseFormatter<Formatter<containerMode, Args...>,
containerMode,
Args...>;
template <class... A> template <class... A>
friend Formatter<false, A...> format(StringPiece fmt, A&&... arg); friend Formatter<false, A...> format(StringPiece fmt, A&&... arg);
template <class... A> template <class... A>
...@@ -181,8 +218,9 @@ std::ostream& operator<<(std::ostream& out, ...@@ -181,8 +218,9 @@ std::ostream& operator<<(std::ostream& out,
/** /**
* Formatter objects can be written to stdio FILEs. * Formatter objects can be written to stdio FILEs.
*/ */
template<bool containerMode, class... Args> template <class Derived, bool containerMode, class... Args>
void writeTo(FILE* fp, const Formatter<containerMode, Args...>& formatter); void writeTo(FILE* fp,
const BaseFormatter<Derived, containerMode, Args...>& formatter);
/** /**
* Create a formatter object. * Create a formatter object.
...@@ -384,10 +422,14 @@ void formatNumber(StringPiece val, int prefixLen, FormatArg& arg, ...@@ -384,10 +422,14 @@ void formatNumber(StringPiece val, int prefixLen, FormatArg& arg,
* formatString(fmt.str(), arg, cb); but avoids creating a temporary * formatString(fmt.str(), arg, cb); but avoids creating a temporary
* string if possible. * string if possible.
*/ */
template <class FormatCallback, bool containerMode, class... Args> template <class FormatCallback,
void formatFormatter(const Formatter<containerMode, Args...>& formatter, class Derived,
FormatArg& arg, bool containerMode,
FormatCallback& cb); class... Args>
void formatFormatter(
const BaseFormatter<Derived, containerMode, Args...>& formatter,
FormatArg& arg,
FormatCallback& cb);
} // namespace format_value } // namespace format_value
...@@ -413,6 +455,19 @@ void formatFormatter(const Formatter<containerMode, Args...>& formatter, ...@@ -413,6 +455,19 @@ void formatFormatter(const Formatter<containerMode, Args...>& formatter,
* empty string) * empty string)
*/ */
namespace detail {
template <class T, class Enable = void>
struct IsFormatter : public std::false_type {};
template <class T>
struct IsFormatter<
T,
typename std::enable_if<
std::is_same<typename T::IsFormatter, detail::FormatterTag>::value>::
type> : public std::true_type {};
} // folly::detail
} // namespace folly } // namespace folly
#include <folly/Format-inl.h> #include <folly/Format-inl.h>
......
...@@ -25,6 +25,8 @@ ...@@ -25,6 +25,8 @@
#include <folly/dynamic.h> #include <folly/dynamic.h>
#include <folly/json.h> #include <folly/json.h>
#include <string>
using namespace folly; using namespace folly;
template <class Uint> template <class Uint>
...@@ -379,6 +381,58 @@ TEST(Format, BogusFormatString) { ...@@ -379,6 +381,58 @@ TEST(Format, BogusFormatString) {
EXPECT_THROW(sformatChecked("{0[test}"), std::exception); EXPECT_THROW(sformatChecked("{0[test}"), std::exception);
} }
template <bool containerMode, class... Args>
class TestExtendingFormatter;
template <bool containerMode, class... Args>
class TestExtendingFormatter
: public BaseFormatter<TestExtendingFormatter<containerMode, Args...>,
containerMode,
Args...> {
private:
explicit TestExtendingFormatter(StringPiece& str, Args&&... args)
: BaseFormatter<TestExtendingFormatter<containerMode, Args...>,
containerMode,
Args...>(str, std::forward<Args>(args)...) {}
template <size_t K, class Callback>
void doFormatArg(FormatArg& arg, Callback& cb) const {
std::string result;
auto appender = [&result](StringPiece s) {
result.append(s.data(), s.size());
};
std::get<K>(this->values_).format(arg, appender);
result = sformat("{{{}}}", result);
cb(StringPiece(result));
}
friend class BaseFormatter<TestExtendingFormatter<containerMode, Args...>,
containerMode,
Args...>;
template <class... A>
friend std::string texsformat(StringPiece fmt, A&&... arg);
};
template <class... Args>
std::string texsformat(StringPiece fmt, Args&&... args) {
return TestExtendingFormatter<false, Args...>(
fmt, std::forward<Args>(args)...).str();
}
TEST(Format, Extending) {
EXPECT_EQ(texsformat("I {} brackets", "love"), "I {love} brackets");
EXPECT_EQ(texsformat("I {} nesting", sformat("really {}", "love")),
"I {really love} nesting");
EXPECT_EQ(
sformat("I also {} nesting", texsformat("have an {} for", "affinity")),
"I also have an {affinity} for nesting");
EXPECT_EQ(texsformat("Extending {} in {}",
texsformat("a {}", "formatter"),
"another formatter"),
"Extending {a {formatter}} in {another formatter}");
}
int main(int argc, char *argv[]) { int main(int argc, char *argv[]) {
testing::InitGoogleTest(&argc, argv); testing::InitGoogleTest(&argc, argv);
gflags::ParseCommandLineFlags(&argc, &argv, true); gflags::ParseCommandLineFlags(&argc, &argv, true);
......
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