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

[Zachary Turner via llvm-commits]

It may in fact be better to remove StringRef.getAsInteger.  But that is
quite a large change, and it will involve inverting the conditional at
every single call site, which will be high risk.

On Mon, May 15, 2017 at 9:59 AM Zachary Turner <zturner at google.com> wrote:

> 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/98e71555/attachment.html>