[PATCH] D25587: Introduce llvm FormatVariadic

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 17:26:16 PDT 2016


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>



More information about the llvm-commits mailing list