Stockfish
Stockfish copied to clipboard
Fixed 'possible loss of data' warnings
Rationale for change
These code changes address places where variables of higher size are assigning to variables of lower size, which generate compiler warning as a potential place for loss of data when a source value is higher than the target variable can hold. The warnings are given by MSVC (Visual Studio 2019 and 2022) with default compile options.
List of changes
On src/misc.cpp - the sqrtl() (for "long double") is not needed since the result is stored to just "double"; otherwise r should have been defined as "long double", anyway, this does not make any difference in the debug output.
On src/search.cpp - the complexityAverage.value() returns data of type "int64_t", therefore, the variable should also be of type "int64_t", it is later stored into a variable of type "double" anyway after a division, so should not be of any problem.
On src/syzygy/tbprobe.cpp - we first cast unsigned size_t "base64.size()" to signed int "base64_size", and then we are able to subtract 2 to avoid unsigned overflow. Then we use consistent type of the for loop variable, i.e. "int" everywhere.
Fishtest results
https://tests.stockfishchess.org/tests/view/640e1d6165775d3b539cc95d
LLR: -0.12 (-2.94,2.94) <0.00,2.00>
Total: 1000 W: 265 L: 274 D: 461
Ptnml(0-2): 3, 112, 278, 105, 2
We use type(expr)
format for primitive type castings. static_cast
seems not necessary.
@MinetaS The compiler gave warning on
uint32_t k = uint32_t(idx / d->span);
Therefore I changed it to
uint32_t k = static_cast<uint32_t>(idx / d->span);
Could you show me what the warning message is? We are already using such convention across the entire source code.
@MinetaS - here they are:
\src\misc.cpp(357,53): warning C5219: implicit conversion from 'int64_t' to 'double', possible loss of data
\src\misc.cpp(378,22): warning C4244: 'initializing': conversion from 'long double' to 'double', possible loss of data
\src\misc.cpp(387,22): warning C4244: 'initializing': conversion from 'long double' to 'double', possible loss of data
\src\search.cpp(235,44): warning C4244: 'argument': conversion from 'double' to 'int', possible loss of data
\src\search.cpp(305,36): warning C4244: 'argument': conversion from 'double' to 'int', possible loss of data
\src\search.cpp(475,26): warning C4244: 'initializing': conversion from 'int64_t' to 'int', possible loss of data
\src\syzygy\tbprobe.cpp(544,43): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
\src\syzygy\tbprobe.cpp(1004,5): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
There are the following warnings in the current (today's) master in this file. The MSVC2022 is set with default settings.
src\syzygy\tbprobe.cpp(544,43): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
src\syzygy\tbprobe.cpp(1004,5): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
src\thread.cpp(202,68): warning C4244: '=': conversion from '_Ty' to 'int', possible loss of data
Ok, so this is not a problem of the syntax but rather a compiler issue (MSVC).
The first snippet of code converts uint64_t
to uint32_t
implicitly, and the second one has explicit conversion operator.
int main() {
uint64_t x;
std::cin >> x;
uint32_t y = x;
std::cout << y << std::endl;
return 0;
}
int main() {
uint64_t x;
std::cin >> x;
uint32_t y = uint32_t(x);
std::cout << y << std::endl;
return 0;
}
...and gcc/clang command line is:
g++ test.cc -Wconversion -o test
The first version of code leads to a warning with both compilers.
gcc:
test.cc: In function ‘int main()’:
test.cc:7:18: warning: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Wconversion]
7 | uint32_t y = x;
|
clang:
test.cc:7:18: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32]
uint32_t y = x;
~ ^
But the second version has no warnings. This means that programmer showed their intention explicitly in the code, and both compilers consider it understandable. From my perspective, MSVC complaining of not using static_cast
about simple primitive conversions is quite hysteric. (not to mention it obviously deteriorates readability)
So, I believe suppressing warnings in this way is not the best option we can have.
Other changes not involving static_cast
look good to me. Please consider submitting a new PR or updating this one.
partially merged via https://github.com/official-stockfish/Stockfish/commit/a9a0dbbcd0749b4e6255c7e9a17f19cffedaa531, thanks!