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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 25 13:53:53 PDT 2018


NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163
+
+    switch (FloatingSize) {
+      case 64:
----------------
donat.nagy wrote:
> NoQ wrote:
> > Continuing the float semantics discussion on the new revision - Did you consider `llvm::APFloat`? (http://llvm.org/doxygen/classllvm_1_1APFloat.html) This looks like something that you're trying to re-implement.
> This switch statement essentially fulfills two functions: it maps QualTypes to standardized IEEE floating point types and it immediately maps the standardized IEEE types to  their precision values.
> 
> The difficulty is that I don't think that the first map is available as a public function in the clang libraries. This means that although a switch over the bit width of the floating point type is certainly inelegant, I cannot avoid it.
> 
> The second map is available in the APFloat library (via the llvm::fltSemantics constants, which describe the standard IEEE types). However, this map is extremely stable (e.g. I don't think that the binary structure of the IEEE double type will ever change), so I think that re-implementing it is justified by the fact that it yields shorter and cleaner code. However, as I had noted previously, I am open to using the llvm::fltSemantics constants to implement this mapping.
> 
> As an additional remark, my current code supports the _Float16 type, which is currently not supported by the APFloat/fltSemantics library. If we decide to use the llvm::fltSemantics constants, then we must either extend the APFloat library or apply some workarounds for this case.  
> 
> The difficulty is that I don't think that the first map is available as a public function in the clang libraries. This means that although a switch over the bit width of the floating point type is certainly inelegant, I cannot avoid it.

`fltSemantics` are not just enum constants, [[ http://llvm.org/doxygen/structllvm_1_1fltSemantics.html | they contain a lot of fancy data ]]:
```
   static const fltSemantics semIEEEhalf = {15, -14, 11, 16};
   static const fltSemantics semIEEEsingle = {127, -126, 24, 32};
   static const fltSemantics semIEEEdouble = {1023, -1022, 53, 64};
   static const fltSemantics semIEEEquad = {16383, -16382, 113, 128};
   static const fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};
```


Repository:
  rC Clang

https://reviews.llvm.org/D52730





More information about the cfe-commits mailing list