Good suggestion.  I wrote all the tests *before* adding the user defined literals.  Check the very last test.  That's how most people will probably use it in practice.<br><br>I will rewrite all the tests in terms of UDLs<br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 13, 2016 at 5:26 PM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I don't think I'll have bandwidth for a full review, but a couple of high-level comments off-the-cuff:<br class="gmail_msg">
<br class="gmail_msg">
- create_formatted_string and format_string_object_base don't really following the LLVM coding conventions.  Should be createFormattedString and FormatStringObjectBase.  There may be other examples?<br class="gmail_msg">
<br class="gmail_msg">
- From what I could see in the unittests, this looks fairly verbose to use vs. the current llvm::format.  Maybe I missed it, but it would be nice for the *safe* one to be easier to type.  Maybe rename llvm::format to llvm::formatPrintf or formatLegacy or something, and make this llvm::format?  Or is there a nice short name for this that doesn't conflict with llvm::format?<br class="gmail_msg">
<br class="gmail_msg">
- It might be nice to add 1 or 2 uses in the actual code to see it in practice.<br class="gmail_msg">
<br class="gmail_msg">
Excited for this!<br class="gmail_msg">
<br class="gmail_msg">
> On 2016-Oct-13, at 17:19, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> zturner created this revision.<br class="gmail_msg">
> zturner added reviewers: chandlerc, timshen, dblaikie, dexonsmith, silvas, beanz.<br class="gmail_msg">
> zturner added a subscriber: llvm-commits.<br class="gmail_msg">
> Herald added subscribers: modocache, mgorny.<br class="gmail_msg">
><br class="gmail_msg">
> As discussed on the mailing list, here is my initial pass at the variadic formatting library.<br class="gmail_msg">
><br class="gmail_msg">
> Formatters are provided for integral, character, string, and pointer types.  More complex formatters such as those required for `chrono` classes are not yet provided, but they are lower priority.<br class="gmail_msg">
><br class="gmail_msg">
> Note that this patch is blocked on <a href="https://reviews.llvm.org/D25497" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D25497</a>, so while I welcome reviews to this any time, it can't go in until that one is approved.<br class="gmail_msg">
><br class="gmail_msg">
> Adding a bunch of people that expressed interest in this from the list.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> <a href="https://reviews.llvm.org/D25587" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D25587</a><br class="gmail_msg">
><br class="gmail_msg">
> Files:<br class="gmail_msg">
>  include/llvm/ADT/STLExtras.h<br class="gmail_msg">
>  include/llvm/Support/FormatProviders.h<br class="gmail_msg">
>  include/llvm/Support/FormatVariadic.h<br class="gmail_msg">
>  include/llvm/Support/FormatVariadicDetails.h<br class="gmail_msg">
>  include/llvm/Support/NativeFormatting.h<br class="gmail_msg">
>  include/llvm/Support/YAMLTraits.h<br class="gmail_msg">
>  include/llvm/Support/raw_ostream.h<br class="gmail_msg">
>  lib/Support/CMakeLists.txt<br class="gmail_msg">
>  lib/Support/FormatVariadic.cpp<br class="gmail_msg">
>  lib/Support/NativeFormatting.cpp<br class="gmail_msg">
>  lib/Support/raw_ostream.cpp<br class="gmail_msg">
>  unittests/Support/CMakeLists.txt<br class="gmail_msg">
>  unittests/Support/FormatVariadicTest.cpp<br class="gmail_msg">
><br class="gmail_msg">
> <D25587.74602.patch><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>