llvm-readobj will need the ability to print floats for use in
HashHistograms. This adds that functionality to the ScopedPrinter and
JSONScopedPrinter.
Details
- Reviewers
jhenderson - Commits
- rG89359df8ca87: [support] Support printing floats in ScopedPrinter
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/Support/ScopedPrinter.h | ||
---|---|---|
237 | Would it make sense to add a double overload too, whilst you're at it? I think it might be a little confusing having one but not the other. (I can see the argument against it, since it's not going to be used immediately though, so happy either way) | |
llvm/unittests/Support/ScopedPrinterTest.cpp | ||
560 | Perhaps should check infinity too (https://en.cppreference.com/w/cpp/types/numeric_limits/infinity). |
Looks good, but see inline comment for possible suggestion.
llvm/unittests/Support/ScopedPrinterTest.cpp | ||
---|---|---|
592 | Just a thought, but this value is implementation defined ultimately. I'm not suggesting necessarily changing this, but if you don't, you should keep a close eye on CI after landing this in case it doesn't work for a particularly system. The alternative would be to build this string up dynamically, using std::to_string on the same values as the ones passed to printNumber. |
Hi, I think this patch is causing a failure on the clang-ppc64-aix bot here https://lab.llvm.org/buildbot/#/builders/214/builds/6531/steps/6/logs/FAIL__LLVM-Unit__83 . Would you be able to take a look?
so it looks like there are additional platform specific behaviors to how things like infinite are printed (inf vs INF, etc.). @abhina.sreeskantharajan I should be able to forward fix this fairly soon, probably in an hour(commute + meeting), but all that should be required is to adjust the test string the same way we do for max doubles/floats so that it's dynamically created and will thus match on all platforms.
Would it make sense to add a double overload too, whilst you're at it? I think it might be a little confusing having one but not the other. (I can see the argument against it, since it's not going to be used immediately though, so happy either way)