[PATCH] D25587: Introduce llvm FormatVariadic

[Bob Haarman via llvm-commits]

inglorion added inline comments.


================
Comment at: include/llvm/Support/FormatProviders.h:87
+///
+///   ==========================================================================
+///   |  style  |     Meaning          |      Example     | Precision Meaning  |
----------------
zturner wrote:
> inglorion wrote:
> > For styles other than hex and exponential, is there a difference between the uppercase and lowercase specifiers? If not, consider only supporting one so that we get consistent usage in the code base.
> No, but at least for hex, the difference between `0xABC` and `0xabc` is likely to start a heated debate :)  If lots of people feel strongly I can standardize on only one, but in my experience there are times when lowercase looks awkward and times when uppercase looks awkward, so I wouldn't want to force one or the other.
To clarify: I meant supporting only one casing in cases where there is no functional difference between the two. I'm all for supporting both uppercase and lowercase hex, and using uppercase and lowercase format specifiers seems like a great way to do it. I'm less sure about supporting both D and d, N and n. I realize that I attached my comment to the hex handling, which is the most confusing place I could have chosen, given that I'm actually happy with that one. My bad.


================
Comment at: lib/Support/NativeFormatting.cpp:72
+    double D = static_cast<double>(N);
+    if (IsNegative)
+      D = -D;
----------------
zturner wrote:
> inglorion wrote:
> > I don't follow - why do we have to check IsNegative when T is an unsigned type?
> Because we can get here via `write_signed`.  Basically, this means "was the **original** number less than 0".  In `write_signed`, we check if it's less than 0 (hence meaning a negative has to be printed), then massage it to get the absolute value into an unsigned integer, then pass it into this function with `IsNegative=true`.  This allows us to have one function that prints any number, and if `IsNegative` is true it just puts a `-` in front.  Otherwise you end up with 3 or 4 functions which all look exactly the same except for 1-2 lines of code, which is irksome.
Ah, I see. So write_unsigned_impl can be used to write both positive and negative integers, it's just that they're passed in as a sign bit and an unsigned magnitude. I guess I was confused by the name of the function.


https://reviews.llvm.org/D25587