[PATCH] D25587: Introduce llvm FormatVariadic

Pavel Labath via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 1 04:26:39 PDT 2016

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
+  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.


More information about the llvm-commits mailing list