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

[Mehdi AMINI via llvm-commits]

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