[Lldb-commits] [PATCH] D28519: Add format_provider for the Error class

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 11 08:09:26 PST 2017


Official Lgtm
On Wed, Jan 11, 2017 at 4:48 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> labath added a comment.
>
> In https://reviews.llvm.org/D28519#641938, @clayborg wrote:
>
> > You can't add anything extra to the AsCString() since it returns a
> "const char *". You can't return a StringRef because it isn't backed by
> anything. You could return a std::string.
> >
> > My vote would be to leave AsCString() alone and have it just return a
> pointer to the string buffer that it owns, and let the formatv stuff do the
> extra formatting.
>
>
> The way I would add it is to add whatever text we want directly to the
> m_string variable that backs the returned `const char *`. Basically, I
> believe that if a function has a "natural" string conversion function
> (which `AsCString` is) then the **default** formatv conversion should be
> just that (obviously the formatv conversion can do other fancy stuff with
> the format modifiers, but that's another story).
>
> I believe we have converged to keeping this patch as is. If I don't hear
> any comments, I am going to land this tomorrow.
>
>
> https://reviews.llvm.org/D28519
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170111/db81b695/attachment.html>


More information about the lldb-commits mailing list