[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