[PATCH] Teach llvm::format_hex() to support formatting without a prefix '0x'

Zachary Turner zturner at google.com
Sat Jan 24 15:06:17 PST 2015


I think format_hex was chosen because raw_ostream is an STL like class.
There's already format_hex, format_decimal, left_justify, and
right_justify.  Changing format_hex to formatHex breaks consistency.  Given
all this, and that it's kind of a minor issue anyway, I think whether or
not to change the default behavior of format_hex() or how and whether to
rename it should be considered orthogonal to this CL.

On Sat Jan 24 2015 at 10:56:12 AM Sean Silva <chisophugis at gmail.com> wrote:

> On Sat, Jan 24, 2015 at 5:09 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> Fwiw, the default value of 4th arg is true already, so it's unnecessary.
>> As of my latest patch i moved no prefix mode to a separate function, so
>> printing with/without prefix is now
>>
>> OS << format_hex(N, 16) // 0x12345
>> vs.
>> OS << format_hex_no_prefix(N, 16) // 12345
>>
>> It does seem a bit clearer if we change it, but i was worried that it
>> would break external users in ways that would only be visible at runtime,
>> which is much worse than breaking them at compile time
>
>
> If that is a concern, you can change the name to formatHex (no clue why
> format_hex was chosen instead, as that would comply with the coding
> standard better). Comments in raw_ostream.h even refers to it as formatHex.
>
> -- Sean Silva
>
>
>>
>> On Sat, Jan 24, 2015 at 8:17 AM Sean Silva <chisophugis at gmail.com> wrote:
>>
>>> On Fri, Jan 23, 2015 at 10:00 PM, Chandler Carruth <chandlerc at google.com
>>> > wrote:
>>>
>>>>
>>>> On Fri, Jan 23, 2015 at 1:58 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>>
>>>>> Why does format_hex prints out "0x" prefix in the first place? If it
>>>>> doesn't, like printf %x, we don't need an extra parameter or a separate
>>>>> function. It may be too late, though.
>>>>
>>>>
>>>> I would be fine to change all users to just write '0x' into the stream
>>>> first.
>>>>
>>>
>>> +1. Compare:
>>>
>>> OS << format_hex(N, 16, false, true);
>>> vs.
>>> OS << "0x" << format_hex(N, 16);
>>>
>>> -- Sean Silva
>>>
>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150124/ae7ef4d7/attachment.html>


More information about the llvm-commits mailing list