[llvm] r289795 - Simplify format member detection in FormatVariadic
Pavel Labath via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 15 01:40:27 PST 2016
Author: labath
Date: Thu Dec 15 03:40:27 2016
New Revision: 289795
URL: http://llvm.org/viewvc/llvm-project?rev=289795&view=rev
Log:
Simplify format member detection in FormatVariadic
Summary:
This replaces the format member search, which was quite complicated, with a more
direct approach to detecting whether a class should be formatted using the
format-member method. Instead we use a special type llvm::format_adapter, which
every adapter must inherit from. Then the search can be simply implemented with
the is_base_of type trait.
Aside from the simplification, I like this way more because it makes it more
explicit that you are supposed to use this type only for adapter-like
formattings, and the other approach (format_provider overloads) should be used
as a default (a mistake I made when first trying to use this library).
The only slight change in behaviour here is that now choose the format-adapter
branch even if the format member invocation will fail to compile (e.g. because it is a
non-const member function and we are passing a const adapter), whereas
previously we would have gone on to search for format_providers for the type.
However, I think that is actually a good thing, as it probably means the
programmer did something wrong.
Reviewers: zturner, inglorion
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D27679
Modified:
llvm/trunk/docs/ProgrammersManual.rst
llvm/trunk/include/llvm/Support/FormatAdapters.h
llvm/trunk/include/llvm/Support/FormatCommon.h
llvm/trunk/include/llvm/Support/FormatProviders.h
llvm/trunk/include/llvm/Support/FormatVariadic.h
llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
llvm/trunk/unittests/Support/FormatVariadicTest.cpp
Modified: llvm/trunk/docs/ProgrammersManual.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/ProgrammersManual.rst?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/docs/ProgrammersManual.rst (original)
+++ llvm/trunk/docs/ProgrammersManual.rst Thu Dec 15 03:40:27 2016
@@ -331,16 +331,15 @@ There are two ways to customize the form
to extend the mechanism for formatting a type that the library already knows how to
format. For that, we need something else.
-2. Provide a **format adapter** with a non-static format method.
+2. Provide a **format adapter** inheriting from ``llvm::FormatAdapter<T>``.
.. code-block:: c++
namespace anything {
- struct format_int_custom {
- int N;
- explicit format_int_custom(int N) : N(N) {}
- void format(llvm::raw_ostream &Stream, StringRef Style) {
- // Do whatever is necessary to format ``N`` into ``Stream``
+ struct format_int_custom : public llvm::FormatAdapter<int> {
+ explicit format_int_custom(int N) : llvm::FormatAdapter<int>(N) {}
+ void format(llvm::raw_ostream &Stream, StringRef Style) override {
+ // Do whatever is necessary to format ``this->Item`` into ``Stream``
}
};
}
@@ -350,9 +349,8 @@ There are two ways to customize the form
}
}
- If the search for a specialization of ``format_provider<T>`` for the given type
- fails, ``formatv`` will subsequently check the argument for an instance method
- named ``format`` with the signature described above. If so, it will call the
+ If the type is detected to be derived from ``FormatAdapter<T>``, ``formatv``
+ will call the
``format`` method on the argument passing in the specified style. This allows
one to provide custom formatting of any type, including one which already has
a builtin format provider.
Modified: llvm/trunk/include/llvm/Support/FormatAdapters.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatAdapters.h?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatAdapters.h (original)
+++ llvm/trunk/include/llvm/Support/FormatAdapters.h Thu Dec 15 03:40:27 2016
@@ -17,57 +17,58 @@
#include "llvm/Support/raw_ostream.h"
namespace llvm {
-template <typename T> class AdapterBase {
+template <typename T> class FormatAdapter : public detail::format_adapter {
protected:
- explicit AdapterBase(T &&Item) : Item(Item) {}
+ explicit FormatAdapter(T &&Item) : Item(Item) {}
T Item;
+
static_assert(!detail::uses_missing_provider<T>::value,
"Item does not have a format provider!");
};
namespace detail {
-template <typename T> class AlignAdapter : public AdapterBase<T> {
+template <typename T> class AlignAdapter final : public FormatAdapter<T> {
AlignStyle Where;
size_t Amount;
public:
AlignAdapter(T &&Item, AlignStyle Where, size_t Amount)
- : AdapterBase<T>(std::forward<T>(Item)), Where(Where), Amount(Amount) {}
+ : FormatAdapter<T>(std::forward<T>(Item)), Where(Where), Amount(Amount) {}
void format(llvm::raw_ostream &Stream, StringRef Style) {
- auto Wrapper = detail::build_format_wrapper(std::forward<T>(this->Item));
- FmtAlign(Wrapper, Where, Amount).format(Stream, Style);
+ auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item));
+ FmtAlign(Adapter, Where, Amount).format(Stream, Style);
}
};
-template <typename T> class PadAdapter : public AdapterBase<T> {
+template <typename T> class PadAdapter final : public FormatAdapter<T> {
size_t Left;
size_t Right;
public:
PadAdapter(T &&Item, size_t Left, size_t Right)
- : AdapterBase<T>(std::forward<T>(Item)), Left(Left), Right(Right) {}
+ : FormatAdapter<T>(std::forward<T>(Item)), Left(Left), Right(Right) {}
void format(llvm::raw_ostream &Stream, StringRef Style) {
- auto Wrapper = detail::build_format_wrapper(std::forward<T>(this->Item));
+ auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item));
Stream.indent(Left);
- Wrapper.format(Stream, Style);
+ Adapter.format(Stream, Style);
Stream.indent(Right);
}
};
-template <typename T> class RepeatAdapter : public AdapterBase<T> {
+template <typename T> class RepeatAdapter final : public FormatAdapter<T> {
size_t Count;
public:
RepeatAdapter(T &&Item, size_t Count)
- : AdapterBase<T>(std::forward<T>(Item)), Count(Count) {}
+ : FormatAdapter<T>(std::forward<T>(Item)), Count(Count) {}
void format(llvm::raw_ostream &Stream, StringRef Style) {
- auto Wrapper = detail::build_format_wrapper(std::forward<T>(this->Item));
+ auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item));
for (size_t I = 0; I < Count; ++I) {
- Wrapper.format(Stream, Style);
+ Adapter.format(Stream, Style);
}
}
};
@@ -89,4 +90,4 @@ detail::RepeatAdapter<T> fmt_repeat(T &&
}
}
-#endif
\ No newline at end of file
+#endif
Modified: llvm/trunk/include/llvm/Support/FormatCommon.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatCommon.h?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatCommon.h (original)
+++ llvm/trunk/include/llvm/Support/FormatCommon.h Thu Dec 15 03:40:27 2016
@@ -18,12 +18,12 @@ namespace llvm {
enum class AlignStyle { Left, Center, Right };
struct FmtAlign {
- detail::format_wrapper &Wrapper;
+ detail::format_adapter &Adapter;
AlignStyle Where;
size_t Amount;
- FmtAlign(detail::format_wrapper &Wrapper, AlignStyle Where, size_t Amount)
- : Wrapper(Wrapper), Where(Where), Amount(Amount) {}
+ FmtAlign(detail::format_adapter &Adapter, AlignStyle Where, size_t Amount)
+ : Adapter(Adapter), Where(Where), Amount(Amount) {}
void format(raw_ostream &S, StringRef Options) {
// If we don't need to align, we can format straight into the underlying
@@ -32,13 +32,13 @@ struct FmtAlign {
// TODO: Make the format method return the number of bytes written, that
// way we can also skip the intermediate stream for left-aligned output.
if (Amount == 0) {
- Wrapper.format(S, Options);
+ Adapter.format(S, Options);
return;
}
SmallString<64> Item;
raw_svector_ostream Stream(Item);
- Wrapper.format(Stream, Options);
+ Adapter.format(Stream, Options);
if (Amount <= Item.size()) {
S << Item;
return;
@@ -66,4 +66,4 @@ struct FmtAlign {
};
}
-#endif
\ No newline at end of file
+#endif
Modified: llvm/trunk/include/llvm/Support/FormatProviders.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatProviders.h?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatProviders.h (original)
+++ llvm/trunk/include/llvm/Support/FormatProviders.h Thu Dec 15 03:40:27 2016
@@ -394,16 +394,16 @@ public:
auto Begin = V.begin();
auto End = V.end();
if (Begin != End) {
- auto Wrapper =
- detail::build_format_wrapper(std::forward<reference>(*Begin));
- Wrapper.format(Stream, ArgStyle);
+ auto Adapter =
+ detail::build_format_adapter(std::forward<reference>(*Begin));
+ Adapter.format(Stream, ArgStyle);
++Begin;
}
while (Begin != End) {
Stream << Sep;
- auto Wrapper =
- detail::build_format_wrapper(std::forward<reference>(*Begin));
- Wrapper.format(Stream, ArgStyle);
+ auto Adapter =
+ detail::build_format_adapter(std::forward<reference>(*Begin));
+ Adapter.format(Stream, ArgStyle);
++Begin;
}
}
Modified: llvm/trunk/include/llvm/Support/FormatVariadic.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatVariadic.h?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatVariadic.h (original)
+++ llvm/trunk/include/llvm/Support/FormatVariadic.h Thu Dec 15 03:40:27 2016
@@ -71,15 +71,15 @@ protected:
// parameters in a template class that derives from a non-template superclass.
// Essentially, we are converting a std::tuple<Derived<Ts...>> to a
// std::vector<Base*>.
- struct create_wrappers {
+ struct create_adapters {
template <typename... Ts>
- std::vector<detail::format_wrapper *> operator()(Ts &... Items) {
- return std::vector<detail::format_wrapper *>{&Items...};
+ std::vector<detail::format_adapter *> operator()(Ts &... Items) {
+ return std::vector<detail::format_adapter *>{&Items...};
}
};
StringRef Fmt;
- std::vector<detail::format_wrapper *> Wrappers;
+ std::vector<detail::format_adapter *> Adapters;
std::vector<ReplacementItem> Replacements;
static bool consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
@@ -91,7 +91,7 @@ protected:
public:
formatv_object_base(StringRef Fmt, std::size_t ParamCount)
: Fmt(Fmt), Replacements(parseFormatString(Fmt)) {
- Wrappers.reserve(ParamCount);
+ Adapters.reserve(ParamCount);
}
void format(raw_ostream &S) const {
@@ -102,12 +102,12 @@ public:
S << R.Spec;
continue;
}
- if (R.Index >= Wrappers.size()) {
+ if (R.Index >= Adapters.size()) {
S << R.Spec;
continue;
}
- auto W = Wrappers[R.Index];
+ auto W = Adapters[R.Index];
FmtAlign Align(*W, R.Where, R.Align);
Align.format(S, R.Options);
@@ -138,7 +138,7 @@ public:
};
template <typename Tuple> class formatv_object : public formatv_object_base {
- // Storage for the parameter wrappers. Since the base class erases the type
+ // Storage for the parameter adapters. Since the base class erases the type
// of the parameters, we have to own the storage for the parameters here, and
// have the base class store type-erased pointers into this tuple.
Tuple Parameters;
@@ -147,7 +147,7 @@ public:
formatv_object(StringRef Fmt, Tuple &&Params)
: formatv_object_base(Fmt, std::tuple_size<Tuple>::value),
Parameters(std::move(Params)) {
- Wrappers = apply_tuple(create_wrappers(), Parameters);
+ Adapters = apply_tuple(create_adapters(), Parameters);
}
};
@@ -234,12 +234,12 @@ public:
//
template <typename... Ts>
inline auto formatv(const char *Fmt, Ts &&... Vals) -> formatv_object<decltype(
- std::make_tuple(detail::build_format_wrapper(std::forward<Ts>(Vals))...))> {
+ std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...))> {
using ParamTuple = decltype(
- std::make_tuple(detail::build_format_wrapper(std::forward<Ts>(Vals))...));
+ std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...));
return formatv_object<ParamTuple>(
Fmt,
- std::make_tuple(detail::build_format_wrapper(std::forward<Ts>(Vals))...));
+ std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...));
}
} // end namespace llvm
Modified: llvm/trunk/include/llvm/Support/FormatVariadicDetails.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FormatVariadicDetails.h?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FormatVariadicDetails.h (original)
+++ llvm/trunk/include/llvm/Support/FormatVariadicDetails.h Thu Dec 15 03:40:27 2016
@@ -19,83 +19,26 @@ namespace llvm {
template <typename T, typename Enable = void> struct format_provider {};
namespace detail {
-
-class format_wrapper {
+class format_adapter {
protected:
- virtual ~format_wrapper() {}
+ virtual ~format_adapter() {}
public:
- virtual void format(llvm::raw_ostream &S, StringRef Options) = 0;
+ virtual void format(raw_ostream &S, StringRef Options) = 0;
};
-template <typename T> class member_format_wrapper : public format_wrapper {
+template <typename T> class provider_format_adapter : public format_adapter {
T Item;
public:
- explicit member_format_wrapper(T &&Item) : Item(Item) {}
-
- void format(llvm::raw_ostream &S, StringRef Options) override {
- Item.format(S, Options);
- }
-};
-
-template <typename T> class provider_format_wrapper : public format_wrapper {
- T Item;
-
-public:
- explicit provider_format_wrapper(T &&Item) : Item(Item) {}
+ explicit provider_format_adapter(T &&Item) : Item(Item) {}
void format(llvm::raw_ostream &S, StringRef Options) override {
format_provider<typename std::decay<T>::type>::format(Item, S, Options);
}
};
-template <typename T> class missing_format_wrapper;
-
-// Test if T is a class that contains a member function with the signature:
-//
-// void format(raw_ostream &, StringRef);
-//
-// It is assumed T is a non-reference type.
-template <class T, class Enable = void> class has_FormatMember {
-public:
- static bool const value = false;
-};
-
-template <class T>
-class has_FormatMember<T,
- typename std::enable_if<std::is_class<T>::value &&
- std::is_const<T>::value>::type> {
- using CleanT = typename std::remove_volatile<T>::type;
- using Signature_format = void (CleanT::*)(llvm::raw_ostream &S,
- StringRef Options) const;
-
- template <typename U>
- static char test2(SameType<Signature_format, &U::format> *);
-
- template <typename U> static double test2(...);
-
-public:
- static bool const value = (sizeof(test2<CleanT>(nullptr)) == 1);
-};
-
-template <class T>
-class has_FormatMember<
- T, typename std::enable_if<std::is_class<T>::value &&
- !std::is_const<T>::value>::type> {
- using CleanT = typename std::remove_cv<T>::type;
- using Signature_format = void (CleanT::*)(llvm::raw_ostream &S,
- StringRef Options);
-
- template <typename U>
- static char test2(SameType<Signature_format, &U::format> *);
-
- template <typename U> static double test2(...);
-
-public:
- static bool const value =
- (sizeof(test2<CleanT>(nullptr)) == 1) || has_FormatMember<const T>::value;
-};
+template <typename T> class missing_format_adapter;
// Test if format_provider<T> is defined on T and contains a member function
// with the signature:
@@ -122,7 +65,8 @@ template <typename T>
struct uses_format_member
: public std::integral_constant<
bool,
- has_FormatMember<typename std::remove_reference<T>::type>::value> {};
+ std::is_base_of<format_adapter,
+ typename std::remove_reference<T>::type>::value> {};
// Simple template that decides whether a type T should use the format_provider
// based format() invocation. The member function takes priority, so this test
@@ -144,24 +88,23 @@ struct uses_missing_provider
!uses_format_provider<T>::value> {};
template <typename T>
-typename std::enable_if<uses_format_member<T>::value,
- member_format_wrapper<T>>::type
-build_format_wrapper(T &&Item) {
- return member_format_wrapper<T>(std::forward<T>(Item));
+typename std::enable_if<uses_format_member<T>::value, T>::type
+build_format_adapter(T &&Item) {
+ return std::forward<T>(Item);
}
template <typename T>
typename std::enable_if<uses_format_provider<T>::value,
- provider_format_wrapper<T>>::type
-build_format_wrapper(T &&Item) {
- return provider_format_wrapper<T>(std::forward<T>(Item));
+ provider_format_adapter<T>>::type
+build_format_adapter(T &&Item) {
+ return provider_format_adapter<T>(std::forward<T>(Item));
}
template <typename T>
typename std::enable_if<uses_missing_provider<T>::value,
- missing_format_wrapper<T>>::type
-build_format_wrapper(T &&Item) {
- return missing_format_wrapper<T>();
+ missing_format_adapter<T>>::type
+build_format_adapter(T &&Item) {
+ return missing_format_adapter<T>();
}
}
}
Modified: llvm/trunk/unittests/Support/FormatVariadicTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/FormatVariadicTest.cpp?rev=289795&r1=289794&r2=289795&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/FormatVariadicTest.cpp (original)
+++ llvm/trunk/unittests/Support/FormatVariadicTest.cpp Thu Dec 15 03:40:27 2016
@@ -13,33 +13,26 @@
using namespace llvm;
-// Compile-time tests for the uses_format_member template
+// Compile-time tests templates in the detail namespace.
namespace {
-struct ConstFormat {
- void format(raw_ostream &OS, StringRef Opt) const { OS << "ConstFormat"; }
-};
-
-struct Format {
- void format(raw_ostream &OS, StringRef Opt) { OS << "Format"; }
+struct Format : public FormatAdapter<int> {
+ Format(int N) : FormatAdapter<int>(std::move(N)) {}
+ void format(raw_ostream &OS, StringRef Opt) override { OS << "Format"; }
};
using detail::uses_format_member;
+using detail::uses_missing_provider;
static_assert(uses_format_member<Format>::value, "");
static_assert(uses_format_member<Format &>::value, "");
static_assert(uses_format_member<Format &&>::value, "");
-static_assert(!uses_format_member<const Format>::value, "");
-static_assert(!uses_format_member<const Format &>::value, "");
-static_assert(!uses_format_member<const volatile Format>::value, "");
-static_assert(!uses_format_member<const volatile Format &>::value, "");
-
-static_assert(uses_format_member<ConstFormat>::value, "");
-static_assert(uses_format_member<ConstFormat &>::value, "");
-static_assert(uses_format_member<ConstFormat &&>::value, "");
-static_assert(uses_format_member<const ConstFormat>::value, "");
-static_assert(uses_format_member<const ConstFormat &>::value, "");
-static_assert(uses_format_member<const volatile ConstFormat>::value, "");
-static_assert(uses_format_member<const volatile ConstFormat &>::value, "");
+static_assert(uses_format_member<const Format>::value, "");
+static_assert(uses_format_member<const Format &>::value, "");
+static_assert(uses_format_member<const volatile Format>::value, "");
+static_assert(uses_format_member<const volatile Format &>::value, "");
+
+struct NoFormat {};
+static_assert(uses_missing_provider<NoFormat>::value, "");
}
TEST(FormatVariadicTest, EmptyFormatString) {
@@ -535,12 +528,10 @@ TEST(FormatVariadicTest, Range) {
}
TEST(FormatVariadicTest, Adapter) {
- class Negative {
- int N;
-
+ class Negative : public FormatAdapter<int> {
public:
- explicit Negative(int N) : N(N) {}
- void format(raw_ostream &S, StringRef Options) const { S << -N; }
+ explicit Negative(int N) : FormatAdapter<int>(std::move(N)) {}
+ void format(raw_ostream &S, StringRef Options) override { S << -Item; }
};
EXPECT_EQ("-7", formatv("{0}", Negative(7)).str());
@@ -566,25 +557,14 @@ TEST(FormatVariadicTest, ImplicitConvers
EXPECT_EQ("1 2", S2);
}
-TEST(FormatVariadicTest, FormatMember) {
- EXPECT_EQ("Format", formatv("{0}", Format()).str());
+TEST(FormatVariadicTest, FormatAdapter) {
+ EXPECT_EQ("Format", formatv("{0}", Format(1)).str());
- Format var;
+ Format var(1);
EXPECT_EQ("Format", formatv("{0}", var).str());
EXPECT_EQ("Format", formatv("{0}", std::move(var)).str());
// Not supposed to compile
- // const Format cvar{};
+ // const Format cvar(1);
// EXPECT_EQ("Format", formatv("{0}", cvar).str());
}
-
-TEST(FormatVariadicTest, FormatMemberConst) {
- EXPECT_EQ("ConstFormat", formatv("{0}", ConstFormat()).str());
-
- ConstFormat var;
- EXPECT_EQ("ConstFormat", formatv("{0}", var).str());
- EXPECT_EQ("ConstFormat", formatv("{0}", std::move(var)).str());
-
- const ConstFormat cvar{};
- EXPECT_EQ("ConstFormat", formatv("{0}", cvar).str());
-}
More information about the llvm-commits
mailing list