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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

The new addition makes perfect sense, please feel free to commit :) Eventually i guess we'll need to declare where is this checker going and what specific lint rule is this checker designed to check, document this rule, and move it to `optin` once it has no false positives with respect to that rule and warning messages are improved. I suspect that it would need `trackNullOrUndefValue()` aka `trackExpressionValue()` and the currently discussed constraint tracking so that to explain why does the checker thinks that the input value of the cast is constrained to be losing precision on the current path.

> I only wonder if this should be on by default or guarded by a config option.

Off-by-default flags mostly make sense when the newly added feature is somehow more opt-in than the rest of the checker, and for now i don't see any indication of that. If it turns out that this part of the checker has its own specific problems, i guess we can put it behind a flag, otherwise kinda why bother?



================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:158-162
+    unsigned FloatingSize = AC.getTypeSize(DestType);
+    // getAllOneValues returns an APFloat with semantics corresponding to the
+    // bit size given as the first argument; this is the only function in
+    // APFloat.h that maps bit width to semantics.
+    llvm::APFloat Tmp = llvm::APFloat::getAllOnesValue(FloatingSize, true);
----------------
Hmm, so the remaining problem is how to extract float semantics from a float `QualType`? Would `ASTContext::getFloatTypeSemantics(DestType)` make sense?


================
Comment at: test/Analysis/conversion.c:195
+    return r;
+  } else if (b>1<<25) {
+    float f = b; // expected-warning {{Loss of precision}}
----------------
Szelethus wrote:
> This too -- how about running clang-format on this file?
Tests are often unformatted and we usually don't care. Additionally, they are almost always violating LLVM's camelCase vs. under_score conventions.


Repository:
  rC Clang

https://reviews.llvm.org/D52730