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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139955