[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