<div dir="ltr">This fix got committed in r240553.<div><br></div><div>Cheers,</div><div>Alex</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-06-24 9:54 GMT-07:00 Alex L <span dir="ltr"><<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Yeah, this seems better. I'll commit a fix.<span class="HOEnZb"><font color="#888888"><div><br></div><div>Alex</div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">2015-06-24 9:16 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
> On 2015 Jun 23, at 11:22, Alex Lorenz <<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.com</a>> wrote:<br>
><br>
> Author: arphaman<br>
> Date: Tue Jun 23 13:22:10 2015<br>
> New Revision: 240436<br>
><br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D240436-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=27jrLxz6JPWl6VcfzsVXMyBqV-1sgxqaeM1Mj8YGuq4&s=-FvxJ4eVnPDD7goxLiPnWZiYOzl0we4n-75g7bdYYFI&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=240436&view=rev</a><br>
> Log:<br>
> ADT: Add a string APSInt constructor.<br>
><br>
> This commit moves the APSInt initialization code that's used by<br>
> the LLLexer class into a new APSInt constructor that constructs<br>
> APSInts from strings.<br>
><br>
> This change is useful for MIR Serialization, as it would allow<br>
> the MILexer class to use the same APSInt initialization as<br>
> LLexer when parsing immediate machine operands.<br>
><br>
> Modified:<br>
>    llvm/trunk/include/llvm/ADT/APSInt.h<br>
>    llvm/trunk/lib/AsmParser/LLLexer.cpp<br>
>    llvm/trunk/lib/Support/APSInt.cpp<br>
>    llvm/trunk/unittests/ADT/APSIntTest.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/ADT/APSInt.h<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_ADT_APSInt.h-3Frev-3D240436-26r1-3D240435-26r2-3D240436-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=27jrLxz6JPWl6VcfzsVXMyBqV-1sgxqaeM1Mj8YGuq4&s=pjPVj4alL1c2W3afP35iyEN7wAKNQC-NbTeHyySI24Q&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APSInt.h?rev=240436&r1=240435&r2=240436&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ADT/APSInt.h (original)<br>
> +++ llvm/trunk/include/llvm/ADT/APSInt.h Tue Jun 23 13:22:10 2015<br>
> @@ -33,6 +33,15 @@ public:<br>
>   explicit APSInt(APInt I, bool isUnsigned = true)<br>
>    : APInt(std::move(I)), IsUnsigned(isUnsigned) {}<br>
><br>
> +  /// Construct an APSInt from a string representation.<br>
> +  ///<br>
> +  /// This constructor interprets the string \p Str using the radix of 10.<br>
> +  /// The interpretation stops at the end of the string. The bit width of the<br>
> +  /// constructed APSInt is determined automatically.<br>
> +  ///<br>
> +  /// \param Str the string to be interpreted.<br>
> +  explicit APSInt(StringRef Str);<br>
> +<br>
>   APSInt &operator=(APInt RHS) {<br>
>     // Retain our current sign.<br>
>     APInt::operator=(std::move(RHS));<br>
><br>
> Modified: llvm/trunk/lib/AsmParser/LLLexer.cpp<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_AsmParser_LLLexer.cpp-3Frev-3D240436-26r1-3D240435-26r2-3D240436-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=27jrLxz6JPWl6VcfzsVXMyBqV-1sgxqaeM1Mj8YGuq4&s=696CRcYILmbccMBHTLzwruyntYhCHEahLnY3Zrrrc84&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLLexer.cpp?rev=240436&r1=240435&r2=240436&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/AsmParser/LLLexer.cpp (original)<br>
> +++ llvm/trunk/lib/AsmParser/LLLexer.cpp Tue Jun 23 13:22:10 2015<br>
> @@ -903,20 +903,7 @@ lltok::Kind LLLexer::LexDigitOrNegative(<br>
>   if (CurPtr[0] != '.') {<br>
>     if (TokStart[0] == '0' && TokStart[1] == 'x')<br>
>       return Lex0x();<br>
> -    unsigned Len = CurPtr-TokStart;<br>
> -    uint32_t numBits = ((Len * 64) / 19) + 2;<br>
> -    APInt Tmp(numBits, StringRef(TokStart, Len), 10);<br>
> -    if (TokStart[0] == '-') {<br>
> -      uint32_t minBits = Tmp.getMinSignedBits();<br>
> -      if (minBits > 0 && minBits < numBits)<br>
> -        Tmp = Tmp.trunc(minBits);<br>
> -      APSIntVal = APSInt(Tmp, false);<br>
> -    } else {<br>
> -      uint32_t activeBits = Tmp.getActiveBits();<br>
> -      if (activeBits > 0 && activeBits < numBits)<br>
> -        Tmp = Tmp.trunc(activeBits);<br>
> -      APSIntVal = APSInt(Tmp, true);<br>
> -    }<br>
> +    APSIntVal = APSInt(StringRef(TokStart, CurPtr - TokStart));<br>
>     return lltok::APSInt;<br>
>   }<br>
><br>
><br>
> Modified: llvm/trunk/lib/Support/APSInt.cpp<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Support_APSInt.cpp-3Frev-3D240436-26r1-3D240435-26r2-3D240436-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=27jrLxz6JPWl6VcfzsVXMyBqV-1sgxqaeM1Mj8YGuq4&s=AGS1ZlcP3rI-vg_YZa5TTXuf8xjFXNwlcFnb4b0NRiM&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APSInt.cpp?rev=240436&r1=240435&r2=240436&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Support/APSInt.cpp (original)<br>
> +++ llvm/trunk/lib/Support/APSInt.cpp Tue Jun 23 13:22:10 2015<br>
> @@ -17,6 +17,25 @@<br>
><br>
> using namespace llvm;<br>
><br>
> +APSInt::APSInt(StringRef Str) {<br>
> +  assert(!Str.empty() && "Invalid string length");<br>
> +<br>
> +  // (Over-)estimate the required number of bits.<br>
> +  unsigned NumBits = ((Str.size() * 64) / 19) + 2;<br>
> +  APInt Tmp(NumBits, Str, /*Radix=*/10);<br>
> +  if (Str[0] == '-') {<br>
> +    unsigned MinBits = Tmp.getMinSignedBits();<br>
> +    if (MinBits > 0 && MinBits < NumBits)<br>
> +      Tmp = Tmp.trunc(MinBits);<br>
> +    *this = APSInt(Tmp, /*IsUnsigned=*/false);<br>
> +    return;<br>
> +  }<br>
> +  unsigned ActiveBits = Tmp.getActiveBits();<br>
> +  if (ActiveBits > 0 && ActiveBits < NumBits)<br>
> +    Tmp = Tmp.trunc(ActiveBits);<br>
> +  *this = APSInt(Tmp, /*IsUnsigned=*/true);<br>
> +}<br>
> +<br>
> void APSInt::Profile(FoldingSetNodeID& ID) const {<br>
>   ID.AddInteger((unsigned) (IsUnsigned ? 1 : 0));<br>
>   APInt::Profile(ID);<br>
><br>
> Modified: llvm/trunk/unittests/ADT/APSIntTest.cpp<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_unittests_ADT_APSIntTest.cpp-3Frev-3D240436-26r1-3D240435-26r2-3D240436-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=27jrLxz6JPWl6VcfzsVXMyBqV-1sgxqaeM1Mj8YGuq4&s=InmwrHr0rjFbx5byiNMt0BrQaXs_2aIP-fiTsKDRtJY&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APSIntTest.cpp?rev=240436&r1=240435&r2=240436&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/ADT/APSIntTest.cpp (original)<br>
> +++ llvm/trunk/unittests/ADT/APSIntTest.cpp Tue Jun 23 13:22:10 2015<br>
> @@ -143,4 +143,23 @@ TEST(APSIntTest, compareValues) {<br>
>   EXPECT_TRUE(APSInt::compareValues(U(8), S(-7).trunc(32)) > 0);<br>
> }<br>
><br>
> +TEST(APSIntTest, FromString) {<br>
> +  EXPECT_EQ(APSInt("1").getExtValue(), 1);<br>
> +  EXPECT_EQ(APSInt("-1").getExtValue(), -1);<br>
> +  EXPECT_EQ(APSInt("0").getExtValue(), 0);<br>
> +  EXPECT_EQ(APSInt("56789").getExtValue(), 56789);<br>
> +  EXPECT_EQ(APSInt("-1234").getExtValue(), -1234);<br>
> }<br>
> +<br>
> +#ifdef GTEST_HAS_DEATH_TEST<br>
> +#ifndef NDEBUG<br>
<br>
</div></div>I think you can merge these checks:<br>
<br>
    #if defined(GTEST_HAS_DEATH_TEST) && !defined(NDEBUG)<br>
<br>
Then you just have a single closing #endif.  Seems a little cleaner<br>
to me.<br>
<div><div><br>
> +<br>
> +TEST(APSIntTest, StringDeath) {<br>
> +  EXPECT_DEATH(APSInt(""), "Invalid string length");<br>
> +  EXPECT_DEATH(APSInt("1a"), "Invalid character in digit string");<br>
> +}<br>
> +<br>
> +#endif<br>
> +#endif<br>
> +<br>
> +} // end anonymous namespace<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>