[PATCH] D29918: Add StringRef::getAsDouble
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 13 17:29:55 PST 2017
mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: llvm/include/llvm/ADT/StringRef.h:567
bool getAsInteger(unsigned Radix, APInt &Result) const;
/// @}
----------------
I'd insert the declaration here, after all the integer related ones.
Also shouldn't we have a `consumeDouble` for symmetry with the integer case?
================
Comment at: llvm/lib/Support/StringRef.cpp:604
+ if (Status != APFloat::opOK) {
+ if (!AllowInexact || Status != APFloat::opInexact)
+ return true;
----------------
zturner wrote:
> mehdi_amini wrote:
> > fuse the two if?
> If I fuse the two if it's going to be wrapped by clang-format anyway, and I thought this made it clearer to read. But I can do it either way, LMK if you still want me to.
Do as you prefer, I don't think we have an official guidelines on this (it'd save the braces though).
https://reviews.llvm.org/D29918
More information about the llvm-commits
mailing list