Reduce the amount of warning (-Wsign-conversion, -Wdouble-promotion)

[Orell Garten]

Hi,

when compiling our code we try to address all warnings as well but blaze generates quite some warnings with regard to sign conversion and double promotion in our case. Especially, it keeps use from compiling with -Werror which would be extremely nice.

Some examples:

blaze/include/blaze/math/expressions/DMatDetExpr.h:343:61: warning: implicit conversion changes signedness: 'blaze::blas_int_t' (aka 'int') to 'unsigned long' [-Wsign-conversion]
    const std::unique_ptr<blas_int_t[]> ipiv( new blas_int_t[n] );

blaze/include/blaze/math/lapack/orgr2.h:147:42: warning: operand of ? changes signedness: 'int' to 'unsigned long' [-Wsign-conversion]
    const size_t offset( ( m < n )?( n - m ):( 0UL ) );

blaze/include/blaze/math/shims/Greater.h:137:23: warning: implicit conversion increases floating-point precision: 'double' to 'long double' [-Wdouble-promotion]
    return ( b - a ) > 1E-10;

We we wondering whether this is by design or has just not been taken care of yet. In our opinion, especially the sign conversion in the allocation could be troublesome if n is for whatever reason negative.

We would volunteer to fix as many of them as possible if that would be something you are interested in as well.

Thanks in advance!

‌

[Klaus Iglberger]

Hi Orell!

Thanks for pointing this out. This is not by design, but unfortunate oversights. We have already included the -Wsign-conversion and -Wdouble-promotion in our test suite and fixed a few of the warnings that you’ve pointed out. However, we did not encounter a lot of warnings, potentially because we’re using different types. In order to make certain that we catch as many conversions as possible, could you please post the complete list of warnings or provide a minimum example? Thanks a lot,

Best regards,

Klaus!

[Orell Garten]

HI Klaus!

Thanks for the fast reaction. I was not really able to reproduce the warnings with a small example, but this is the cleaned output of the CI (only the warnings):

https://pastebin.com/dnkFhPdb

We mostly use StaticVector and StaticMatrix with double, float and uint32_t. If you need anything else just let me know.

Best,

Orell

[Klaus Iglberger]

Hi Orell!

With commit 332b7fb we have hopefully fixed the majority of compiler warnings. Unfortunately we haven’t been able to reproduce the warnings related to blas_int_t (the first of your examples) and therefore suspect that these will still appear. This will require us to do one more pass through the entire LAPACK functionality, since this is the only place where we have to use a mix of int and size_t. We’ll try to do this as soon as possible. Any further feedback is welcome. Thanks,

Best regards,

Klaus!

[Orell Garten]

Hi Klaus,

thanks for the fast fix! Looks good so far. At least locally we don’t seem to get the warning anymore. For the CI we have to wait until the new version comes out.

Best,

Orell

[Klaus Iglberger]

Hi Orell!

Thanks for the feedback. If you don’t get warnings related to sign conversions and double promotions anymore, I would consider this problem as fixed and resolve this issue.

Best regards,

Klaus!

[Orell Garten]

Hi Klaus,

I think you can resolve the issue.

Best,
Orell

[Klaus Iglberger]

Commit 332b7fb fixes the warnings caused by -Wsign-conversion and -Wdouble-promotion. The fix is immediately available via cloning the Blaze repository and will be officially released in Blaze 3.9.