[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

ziangwan marked 9 inline comments as done.
ziangwan added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11430
+        SmallString<32> PrettyTargetValue;
+        TargetFloatValue.toString(PrettyTargetValue,
+          TargetPrecision);
----------------
nickdesaulniers wrote:
> xbolva00 wrote:
> > Can you check my older patch + tests + discussion? 
> > 
> > I had to use other way to get this string..
> > 
> > 
> And I don't think there's a test for this case? Or at least one that checks the printed value?
I have checked your older patch.

IIUC, you take the log of number of precision bits and print the floating point value out. E.g. 2555+E9. I am doing it differently. I print out all the precision bits. E.g. 2555677.000. The reason is that I think it is more clear to print out `changes value from 222222222222 to 222222221312` than `changes value from 222222222222 to 222222221+e3`.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11430
+        SmallString<32> PrettyTargetValue;
+        TargetFloatValue.toString(PrettyTargetValue,
+          TargetPrecision);
----------------
ziangwan wrote:
> nickdesaulniers wrote:
> > xbolva00 wrote:
> > > Can you check my older patch + tests + discussion? 
> > > 
> > > I had to use other way to get this string..
> > > 
> > > 
> > And I don't think there's a test for this case? Or at least one that checks the printed value?
> I have checked your older patch.
> 
> IIUC, you take the log of number of precision bits and print the floating point value out. E.g. 2555+E9. I am doing it differently. I print out all the precision bits. E.g. 2555677.000. The reason is that I think it is more clear to print out `changes value from 222222222222 to 222222221312` than `changes value from 222222222222 to 222222221+e3`.
Test cases are added


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

https://reviews.llvm.org/D64666