[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164
+        // double and possibly long double on some systems
+        RepresentsUntilExp = 53; break;
+      case 32:
----------------
A link to the source of these number would be useful. How are these calculated. Also,  as far as I know the current C++ standard does not require anything about how floating points are represented in an implementation. So it would be great to somehow refer to the representation used by clang rather than hardcoding these values. (Note that I am perfectly fine with warning for implementation defined behavior as the original version also warn for such in case of integers.) 


================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:172
+      default:
+        // larger types, which can represent all integers below 2^64
+        return false;
----------------
Comment should be full sentences with a full stop at the end.


================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:183
+  unsigned CorrectedSrcWidth = AC.getIntWidth(SubType);
+  if (SubType->isSignedIntegerType()) {
+    CorrectedSrcWidth--;
----------------
Do not add redundant braces when we only guard one line. It is perfectly ok when the one statements spans over multiple lines (maybe due to comments).


Repository:
  rC Clang

https://reviews.llvm.org/D52730