[PATCH] D25587: Introduce llvm FormatVariadic

[Pavel Labath via llvm-commits]

labath added a comment.

I am not sure about the use of rvalue references. Could you check that they do the thing you wanted to do?



================
Comment at: include/llvm/Support/FormatAdapters.h:22
+protected:
+  explicit AdapterBase(T &&Item) : Item(Item) {}
+
----------------
This will still call the copy constructor. Did you want to use `Item(std::move(Item))` instead?


================
Comment at: include/llvm/Support/FormatAdapters.h:36
+  AlignAdapter(T &&Item, AlignStyle Where, size_t Amount)
+      : AdapterBase(std::forward<T>(Item)), Where(Where), Amount(Amount) {}
+
----------------
`std::forward` is typically used in contexts where `T` is deduced (here it isn't as it is a class parameter). I am not 100% sure but I don't think it's usage here is correct (and std::move should be used instead). std::forward will only do something if you're expecting someone to explicitly instantiate the class with `T = foo &` and such.


https://reviews.llvm.org/D25587