Pedantic Warning: Loss of precision on implicit conversion

[dnygren]

I observed the following pedantic warnings when running the golay-24-systematic source code through a beta version of Oracle's Parfait source code analysis tool.

Pedantic: Loss of precision on implicit conversion
   Loss of integer precision on implicit conversion [implicit-conversion-integer-precision-loss]:
      implicit conversion loses integer precision: 'int' to 'uint_fast16_t' (aka 'unsigned short')
        at line 207 of Firmware/Vivado/IR3C_Zynq/IR3C_Zynq.sdk/IR3C_Zynq_CC/src/golay-24-systematic.cc.
Pedantic: Loss of precision on implicit conversion
   Loss of integer precision on implicit conversion [implicit-conversion-integer-precision-loss]:
      implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'uint_fast16_t' (aka 'unsigned short')
        at line 215 of Firmware/Vivado/IR3C_Zynq/IR3C_Zynq.sdk/IR3C_Zynq_CC/src/golay-24-systematic.cc.

Here are the lines in question:

uint_fast16_t golay_24_systematic_decode(uint32_t x)
{
...
    // Data contains 1 error, parity contains 1 or 2 errors
    for (uint32_t i = 0; i < 12; i++) {
        uint32_t p3 = p2 ^ golay_matrix[i];

        if (popcount(p3 ^ p) <= 2) {
            return d ^ (1 << (11 - i));   // <---*** Line 207 *** Promotion rules convert values to ints
        }
    }
    // Parity contains 1 error, data contains 1 or 2 errors
    for (uint32_t i = 0; i < 12; i++) {
        uint32_t d3 = d2 ^ golay_matrix[i];

        if (popcount(d3 ^ d) <= 2) {
            return d3;  // <---*** Line 215 *** d3 is uint32_t and function return is a uint_fast16_t
        }
    }

BTW, thank you for providing this source code. It helped me a lot!

[Venemo]

Note that there is a test suite in this repo which verifies that the algorithm is correct for all possible inputs.

I suggest filing a bug report against the analysis tool. Sadly, these tools are usually a PITA to work with... A proper range analysis on these values (even just an analysis on the unsigned upper bound of the return values) would correctly show that in reality there is no loss of precision.

That said, we can change the function return type of uint32_t which should alleviate the warning. What do you think?

[dnygren]

Venemo wrote:

... we can change the function return type of uint32_t which should alleviate the warning. What do you think?

Thank you for the response.

When I call golay_24_systematic_decode() I am doing so like this in my code:

uint16_t first_12bits = golay_24_systematic_decode(first_24bits);
uint16_t second_12bits = golay_24_systematic_decode(second_24bits);

if ((first_12bits == GOLAY_24_SYSTEMATIC_ERROR_RESULT) ||
    (second_12bits == GOLAY_24_SYSTEMATIC_ERROR_RESULT))
{
    Status = EXIT_FAILURE;
} else {
    Status = EXIT_SUCCESS;
}

So I am expecting back a uint16_t that contains the decoded 12 bits not a uint32_t which d3 is. Is Line 215's d3 really an uint16_t because it is derived from ORing with d2, uintfast16_t?

I can make the Line 215 problem go away with: return (uint16_t)d3; // Is this correct?

Regarding the d on Line 207 , the analysis tool vendor believes an integer promotion takes place (since all args are less than rank int) - and since an int can represent the entire range of possible values - the values are converted to int (you'd think unsigned int since d is unsigned - but that's not what the spec says). If an int can't represent the value then unsigned int is used (note that is the fallback when int can't be used - it isn't "signed types are promoted to int and unsigned types are promoted to unsigned int").

I can make the Line 207 problem go away with: return (uint16_t)(d ^ (1 << (11 - i))); // Is this correct?

Anyways your code works fine as-is. I'm happy with it, but since you were kind enough to provide this code, I wanted to let you know about these possible latent issues the tool I used flagged. No good deed goes unpunished! :-)

[Venemo]

May I ask, what are you using this for? Just out of curiosity.

Note that uint_fast16_t may not be the same as uint16_t depending on your target architecture. In fact, I'd suspect that on 32-bit CPUs the uint_fast16_t is actually a 32-bit type. So, uint_fast16_t vs. uint32_t probably makes very little difference, unless you want to run this on a small 8 or 16 bit microcontroller.

I can make the Line 207 problem go away with

Like I said, the correct solution here, is to open a bug report against the source code analysis tool, because there really is no issue here. Only the low 16 bits of each return value are actually written, and the analysis tool should notice that. If you want to just silence the warning, yes you can add some casts or a & 0xff somewhere.

[dnygren]

I've been working on a payload for a scientific spacecraft. NASA has expressed no interest in it so it is going to get mothballed in a few months. Maybe the recent change in management in the USA will free up funding. If we ever get it off the ground I'll let you know.