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

[Sean Silva]

On Sat, Jan 24, 2015 at 11:06 PM, Zachary Turner <zturner at google.com> wrote:

> 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.
>

Sure. Makes sense to me.

-- Sean Silva


>
>
> 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/20150126/65c8bb98/attachment.html>