[Lldb-commits] [PATCH] D139955: [LLDB] Change formatting to use llvm::formatv

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

In D139955#3999521 <https://reviews.llvm.org/D139955#3999521>, @labath wrote:

> In D139955#3999381 <https://reviews.llvm.org/D139955#3999381>, @ayermolo wrote:
>
>> I tried to modify all the places wehre getOffset() is used. Which will later return 64bit instead of 32 bit.
>>
>> Sorry I guess there was miss communication.
>> I am not quite clear in what you are suggesting.
>> The way I did it we now have two distinct interfaces. One for old way that takes in char* and args and applies formatting internally, and a new one which takes in std::string. So all formatting is done on caller side with std::string(llvm::formatv(..)).
>> Are you suggesting to change implementation of
>>
>>   void ReportError(const char *format, ...)
>>         __attribute__((format(printf, 2, 3)));
>>
>> and others to use llvm::formatv under the hood instead?
>
> Yes, that's pretty much it.
>
>> So caller side will remain mostly the same. Except instead of using printf formating it will be formatvv.
>> In that case all of call sites will need to change.
>
> Yes, but in the case of ReportError, most of the callers are in DWARF code anyway, so I think it'd make sense to replace all of them (and some of them are already calling formatv, and then passing the string to this function).
>
> If there are too many callers then you can just make a different function (`ReportErrorv`, or `ReportErrorWithFormatv`, depending on how verbose you feel), and then just change the callers that we care about.

Ah, gotcha. Thanks for elaborating.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139955/new/

https://reviews.llvm.org/D139955