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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits