[PATCH] D82300: [unittest, ADT] Add unit tests for itostr & utostr

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/unittests/ADT/StringExtrasTest.cpp:195-196
+  EXPECT_EQ("1", utostr(1, /*isNeg=*/false));
+  EXPECT_EQ("18446744073709551615", utostr(MaxUint64));
+  EXPECT_EQ("18446744073709551615", utostr(MaxUint64, /*isNeg=*/false));
+
----------------
thopre wrote:
> jhenderson wrote:
> > Here, and in the below cases using the max/min values, I wonder whether it would be more robust to write something like:
> > 
> > ```
> >   EXPECT_EQ(std::to_string(MaxUint64), utostr(MaxUint64));
> >   EXPECT_EQ("-" + std::to_string(MaxUint64), utostr(MaxUint64, /*isNeg=*/true);
> > ```
> >  etc.
> > 
> > I'd expect them to produce identical output, but it also is easier to confirm the expected number. It also removes any concerns about numeric representation (2's complement etc).
> It's what I did originally but I was not sure if it was fine to rely on std::to_string being correct.
I think if we can't rely on standard library functions doing the right thing, we're in trouble...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82300/new/

https://reviews.llvm.org/D82300