Questionable data type in devexplore.cpp template readArray and template read

    template<typename VAL>
    unsigned readArray(VAL *val, unsigned count) const
    {
        epicsUInt32 addr = 0,
                    end  = barsize-offset;
        unsigned i;
        for(i=0; i<count && addr<end; i++, addr+=step)
        {
            epicsUInt32 OV = read<VAL>(addr);
            *val++ = castval<VAL,epicsUInt32>::op(OV);
        }
        return i;
    }

When instatiated with epicsFloat32 in line 155, strange conversions are going on (and some compilers warn)
read<VAL> returns epicsFloat32 which is then cast to epicsUInt32, then back to epicsFloat32.

Also, in template read VAL is copied but not cast to a epicsUInt32, which generates a warning for VAL=epicsFloat32.

I find all this casting between epicsUInt32 and epicsFloat32 across 3 templates quite questionable.

As readraw reads epicsUInt32 and then casts to VAL, there is no reason for read to call readraw<VAL>. It should call readraw<epicsUInt32>. For the same reason, readArray should call read<epicsUInt32> instead of read<VAL>.

This does look strange. It's been 4 years since I added this, and I must admit that I'm having trouble remembering the details. The situation I faced at the time involved a custom device with real ieee floating point registers.

For a device with real floating point registers the code won't work. The registers are always read as integers and stored in epicsUInt32 OV. Casting them to float is of course not the same as reading them as float. That would require a readraw (or read) version which uses for example a union to reinterpret the value as float.

There is a union hiding in the castval<> template. So it does work, at least on Linux. I'm working on adding a test case to demonstrate this.

devlib2/exploreApp/src/devexplore.cpp

Lines 75 to 83 in 5d28689

    
           // value conversion 
        
           //  cast different integer sizes using C rules 
        
           //  type pun int to/from float 
        
           template<typename TO, typename FROM> 
        
           struct castval { static TO op(FROM v) { return v; } }; 
        
           template<typename FROM> 
        
           struct castval<epicsFloat32,FROM> { static epicsFloat32 op(FROM v) {punny32 P; P.ival = v; return P.fval;} }; 
        
           template<typename TO> 
        
           struct castval<TO,epicsFloat32> { static TO op(epicsFloat32 v) {punny32 P; P.fval = v; return P.ival;} };

Of course, I must acknowledge that there are unnecessary casts happening, and that the appearance of correctness may well depend on how GCC optimizes these casts. There is room for improvement.

Oops. I missed that. Sorry.

I may have addressed this with 614e0a8. Do you still see this warning? If so, which warning is it? It would help for me to know what to look for in eg. the appveyor output.

I used to get the warnings from the Microsoft compiler (in both, 32 bit and 64 build). It is very sensitive to potential data loss due to implicit casting, e.g. 32 bit integer to 32 bit float.

/opt/wine-msvc-2017/bin/x86/cl -EHsc -GR            -DUSE_TYPED_RSET -I../../../common     -nologo -FC -D__STDC__=0 -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE    -Ox -GL -Oy-   -W3 -w44355 -w44344 -w44251     -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING      -MD -DEPICS_BUILD_DLL -DEPICS_CALL_DLL -TP  -I. -I../O.Common -I. -I. -I.. -I../../../include/compiler/msvc -I../../../include/os/WIN32 -I../../../include -I/usr/local/epics/base-7.0.4.1/include/compiler/msvc -I/usr/local/epics/base-7.0.4.1/include/os/WIN32 -I/usr/local/epics/base-7.0.4.1/include        -c ../devexplore.cpp
devexplore.cpp
z:\afs\psi.ch\group\8211\dirk\git\devlib2\exploreapp\src\devexplore.cpp(154): warning C4244: 'initializing': conversion from 'VAL' to 'epicsUInt32', possible loss of data
        with
        [
            VAL=epicsFloat32
        ]
z:\afs\psi.ch\group\8211\dirk\git\devlib2\exploreapp\src\devexplore.cpp(475): note: see reference to function template instantiation 'unsigned int `anonymous-namespace'::priv::readArray<epicsFloat32>(VAL *,unsigned int) const' being compiled
        with
        [
            VAL=epicsFloat32
        ]
z:\afs\psi.ch\group\8211\dirk\git\devlib2\exploreapp\src\devexplore.cpp(143): warning C4244: 'return': conversion from 'epicsUInt32' to 'VAL', possible loss of data
        with
        [
            VAL=epicsFloat32
        ]
z:\afs\psi.ch\group\8211\dirk\git\devlib2\exploreapp\src\devexplore.cpp(154): note: see reference to function template instantiation 'VAL `anonymous-namespace'::priv::read<VAL>(epicsUInt32) const' being compiled
        with
        [
            VAL=epicsFloat32
        ]
z:\afs\psi.ch\group\8211\dirk\git\devlib2\exploreapp\src\devexplore.cpp(475): note: see reference to function template instantiation 'unsigned int `anonymous-namespace'::priv::readArray<epicsFloat32>(VAL *,unsigned int) const' being compiled
        with
        [
            VAL=epicsFloat32
        ]

I had as well seen this warning from g++ 3.4.3

../devexplore.cpp: In member function `epicsUInt32 <unnamed>::priv::readraw(epicsUInt32) const':
../devexplore.cpp:118: warning: converting of negative value `-0x00000000000000001' to `epicsUInt32'
../devexplore.cpp: In member function `unsigned int <unnamed>::priv::readArray(VAL*, unsigned int) const [with VAL = epicsFloat32]':
../devexplore.cpp:475:   instantiated from here
../devexplore.cpp:154: warning: converting to `epicsUInt32' from `epicsFloat32'

Both are gone now

	// value conversion
	// cast different integer sizes using C rules
	// type pun int to/from float
	template<typename TO, typename FROM>
	struct castval { static TO op(FROM v) { return v; } };
	template<typename FROM>
	struct castval<epicsFloat32,FROM> { static epicsFloat32 op(FROM v) {punny32 P; P.ival = v; return P.fval;} };
	template<typename TO>
	struct castval<TO,epicsFloat32> { static TO op(epicsFloat32 v) {punny32 P; P.fval = v; return P.ival;} };