[PATCH] D25587: Introduce llvm FormatVariadic

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 17:29:07 PDT 2016


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.

I will rewrite all the tests in terms of UDLs
On Thu, Oct 13, 2016 at 5:26 PM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> I don't think I'll have bandwidth for a full review, but a couple of
> high-level comments off-the-cuff:
>
> - 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?
>
> - 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?
>
> - It might be nice to add 1 or 2 uses in the actual code to see it in
> practice.
>
> Excited for this!
>
> > On 2016-Oct-13, at 17:19, Zachary Turner <zturner at google.com> wrote:
> >
> > zturner created this revision.
> > zturner added reviewers: chandlerc, timshen, dblaikie, dexonsmith,
> silvas, beanz.
> > zturner added a subscriber: llvm-commits.
> > Herald added subscribers: modocache, mgorny.
> >
> > As discussed on the mailing list, here is my initial pass at the
> variadic formatting library.
> >
> > 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.
> >
> > Note that this patch is blocked on https://reviews.llvm.org/D25497, so
> while I welcome reviews to this any time, it can't go in until that one is
> approved.
> >
> > Adding a bunch of people that expressed interest in this from the list.
> >
> >
> > https://reviews.llvm.org/D25587
> >
> > Files:
> >  include/llvm/ADT/STLExtras.h
> >  include/llvm/Support/FormatProviders.h
> >  include/llvm/Support/FormatVariadic.h
> >  include/llvm/Support/FormatVariadicDetails.h
> >  include/llvm/Support/NativeFormatting.h
> >  include/llvm/Support/YAMLTraits.h
> >  include/llvm/Support/raw_ostream.h
> >  lib/Support/CMakeLists.txt
> >  lib/Support/FormatVariadic.cpp
> >  lib/Support/NativeFormatting.cpp
> >  lib/Support/raw_ostream.cpp
> >  unittests/Support/CMakeLists.txt
> >  unittests/Support/FormatVariadicTest.cpp
> >
> > <D25587.74602.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161014/a8bb2bbd/attachment.html>


More information about the llvm-commits mailing list