Support formatting formatv_objects.

[sammccall]

Summary

Support formatting formatv_objects.

While here, fix documentation about member-formatters, and attempted
perfect-forwarding (I think).

[zturner]

include/llvm/Support/FormatVariadicDetails.h

34

This looks incorrect? T is not deduced here, so std::forward<T>(Item) is not using a forwarding reference. I think the code before was correct, was it causing a problem?

[sammccall]

include/llvm/Support/FormatVariadicDetails.h

34

This is an attempt to make it possible to pass an rvalue of a move-only type to formatv, e.g. formatv("{0}", formatv("{0}", 1)).

Currently this doesn't work: if an rvalue Foo is passed, build_format_adapter infers T = Foo, and instantiates provider_format_adapter<Foo>, and passes a Foo&&.
The Item member has type Foo, and gets initialized with an lvalue reference (the Item parameter has of type Foo&&, but using Item yields an lvalue: http://www.codesynthesis.com/~boris/blog/2012/03/06/rvalue-reference-pitfalls/). Therefore this calls the copy-constructor, which is unneccesary for movable types and fails to compile for move-only types.

Adding std::forward here is equivalent to std::move if we originally passed an rvalue to formatv, and is no change otherwise.

T is not deduced here

Right, this is a bit unusual, but I think still correct. My intuition is build_format_adapter deduces T such that T&& is a forwarding reference, and then passes T explicitly - T&& is still the right forwarding reference type if we want to forward a second time, because it's the same type as the first time.

If this seems confusing, we could make Item public and treat it as an aggregate: build_format_adapter would do provider_format_adapter<T>{std::forward<T>(Item)}. I think it amounts to the same thing.

[sammccall]

Add test counting copies and moves for movable objects.

include/llvm/Support/FormatVariadic.h

 // to compute the value of the parameter, and you would otherwise have had to
 // save it to a temporary.
 //
 // ===Formatter Search===
 //
 // For a given parameter of type T, the following steps are executed in order
 // until a match is found:
 //
-//   1. If the parameter is of class type, and contains a method
-//      void format(raw_ostream &Stream, StringRef Options)
-//      Then this method is invoked to produce the formatted output.  The
+//   1. If the parameter is of class type, and inherits from format_adapter,
+//      Then format() is invoked on it to produce the formatted output.  The
 //      implementation should write the formatted text into `Stream`.
 //   2. If there is a suitable template specialization of format_provider<>
 //      for type T containing a method whose signature is:
 //      void format(const T &Obj, raw_ostream &Stream, StringRef Options)
 //      Then this method is invoked as described in Step 1.
 //
 // If a match cannot be found through either of the above methods, a compiler
 // error is generated.
inline auto formatv(const char *Fmt, Ts &&... Vals) -> formatv_object<decltype(
     std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...))> {
   using ParamTuple = decltype(
       std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...));
   return formatv_object<ParamTuple>(
       Fmt,
       std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...));
 }
 
+// Allow a formatv_object to be formatted (no options supported).
+template <typename T> struct format_provider<formatv_object<T>> {
+  static void format(const formatv_object<T> &V, raw_ostream &OS, StringRef) {
+    OS << V;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_FORMATVARIADIC_H

include/llvm/Support/FormatVariadicDetails.h

 public:
   virtual void format(raw_ostream &S, StringRef Options) = 0;
 };
 
 template <typename T> class provider_format_adapter : public format_adapter {
   T Item;
 
 public:
-  explicit provider_format_adapter(T &&Item) : Item(Item) {}
+  explicit provider_format_adapter(T &&Item) : Item(std::forward<T>(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_adapter;
 

unittests/Support/FormatVariadicTest.cpp

TEST(FormatVariadicTest, FormatAdapter) {
   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(1);
   // EXPECT_EQ("Format", formatv("{0}", cvar).str());
 }
+
+TEST(FormatVariadicTest, FormatFormatvObject) {
+  EXPECT_EQ("Format", formatv("F{0}t", formatv("o{0}a", "rm")).str());
+  EXPECT_EQ("[   ! ]", formatv("[{0,+5}]", formatv("{0,-2}", "!")).str());
+}
+
+namespace {
+struct Recorder {
+  int Copied = 0, Moved = 0;
+  Recorder() = default;
+  Recorder(const Recorder &Copy) : Copied(1 + Copy.Copied), Moved(Copy.Moved) {}
+  Recorder(const Recorder &&Move)
+      : Copied(Move.Copied), Moved(1 + Move.Moved) {}
+};
+} // namespace
+template <> struct llvm::format_provider<Recorder> {
+  static void format(const Recorder &R, raw_ostream &OS, StringRef style) {
+    OS << R.Copied << "C " << R.Moved << "M";
+  }
+};
+
+TEST(FormatVariadicTest, CopiesAndMoves) {
+  Recorder R;
+  EXPECT_EQ("0C 0M", formatv("{0}", R).str());
+  EXPECT_EQ("0C 3M", formatv("{0}", std::move(R)).str());
+  EXPECT_EQ("0C 3M", formatv("{0}", Recorder()).str());
+  EXPECT_EQ(0, R.Copied);
+  EXPECT_EQ(0, R.Moved);
+}