donat.nagy added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163
+
+    switch (FloatingSize) {
+      case 64:
----------------
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.  



Repository:
  rC Clang

https://reviews.llvm.org/D52730



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to