[PATCH] D24778: Add `StringRef::consumeInteger`

[Zachary Turner via llvm-commits]

I imagine we could just change this function to return Error and everything
would just work. I might try it if i get some cycles
On Thu, Sep 22, 2016 at 11:56 AM Mehdi AMINI <mehdi.amini at apple.com> wrote:

> mehdi_amini added inline comments.
>
> ================
> Comment at: llvm/trunk/lib/Support/StringRef.cpp:384
> @@ -384,3 +383,3 @@
>    // Empty strings (after the radix autosense) are invalid.
>    if (Str.empty()) return true;
>
> ----------------
> amccarth wrote:
> > `true` means invalid?!  Wow!  I did not see that coming.
> >
> > I believe this check is unnecessary now, since this patch adds a check
> near the end to see if any characters were consumed.
> This is a widespread convention in LLVM APIs that returning "true" means
> an error occurred (and I'm always confused).
>
> The new Error class style settles this issue, we should just be more
> consistent on using it.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24778
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160922/d4ba8cb7/attachment.html>