[llvm] r303011 - [StringExtras] Add llvm::to_integer.

[Zachary Turner via llvm-commits]

My commit message may have been unclear.  "This function" refers to the NEW
function.  The new function does in fact return true on success.  It's
StringRef::getAsInteger that doesn't, that's the mistake I was referring to.

On Mon, May 15, 2017 at 9:57 AM David Blaikie <dblaikie at gmail.com> wrote:

> On Sun, May 14, 2017 at 10:24 AM Zachary Turner via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: zturner
>> Date: Sun May 14 12:11:05 2017
>> New Revision: 303011
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=303011&view=rev
>> Log:
>> [StringExtras] Add llvm::to_integer.
>>
>> This is a very thin wrapper around StringRef::getAsInteger.
>> It serves three purposes.
>>
>> 1) It allows a cleaner syntax when you have something other than
>>    a StringRef - for example, a std::string or an llvm::SmallString.
>>    Previously, in this case you would have to write something like:
>>       StringRef(MyStr).getAsInteger(0, Result)
>>    by explicitly constructing a temporary StringRef.  This can be
>>    done implicitly however with the new function by just writing:
>>       to_integer(MyStr, ...).
>>
>
> For this ^ it seems like it'd be better, maybe, to remove the member
> version and just have the non-member version?
>
>
>> 2) Correcting the travesty that is getAsInteger's return value.
>>    This function returns true on success, and false on failure.
>>    While this may cause confusion with people familiar with the
>>    getAsInteger API, there seems to be widespread agreement that
>>    the return semantics of getAsInteger was a mistake.
>>
>
> ^ this might be less obvious. LLVM had/has a long history of that
> true-on-success return bool in some parts of the codebase... :/
>
> Though I guess there's probably enough inconsistency there these days that
> maybe it's fine to lean more heavily towards non-zero (true) on failure.
>
>
>> 3) It allows the Radix to be deduced as a default argument by
>>    putting it last in the parameter list.  Most uses of getAsInteger
>>    pass 0 for the first argument.  With this syntax it can just be
>>    omitted.
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/StringExtras.h
>>
>> Modified: llvm/trunk/include/llvm/ADT/StringExtras.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringExtras.h?rev=303011&r1=303010&r2=303011&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/StringExtras.h (original)
>> +++ llvm/trunk/include/llvm/ADT/StringExtras.h Sun May 14 12:11:05 2017
>> @@ -106,6 +106,13 @@ static inline std::string fromHex(String
>>    return Output;
>>  }
>>
>> +/// \brief Convert the string \p S to an integer of the specified type
>> using
>> +/// the radix \p Base.  If \p Base is 0, auto-detects the radix.
>> +/// Returns true if the number was successfully converted, false
>> otherwise.
>> +template <typename N> bool to_integer(StringRef S, N &Num, unsigned Base
>> = 0) {
>> +  return !S.getAsInteger(Base, Num);
>> +}
>> +
>>  static inline std::string utostr(uint64_t X, bool isNeg = false) {
>>    char Buffer[21];
>>    char *BufPtr = std::end(Buffer);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170515/6bc6d673/attachment.html>