[benchmark] build for QNX failure. Patch suggestion

[Arech]

Operating system

host: Ubuntu20.04 cross-compiling target: QNX710

Compiler

ntoaarch64-g++ from QNX SDP 710

Steps to reproduce the behavior

Crosscompile to QNX and observe build failure due to unaddressed old-style C-casts

Failure logs

FAILED: src/CMakeFiles/benchmark.dir/sysinfo.cc.o
qnx-sdp/qnx710/host/linux/x86_64/usr/bin/ntoaarch64-g++ --sysroot=qnx-sdp/qnx710/target/qnx7 -DBENCHMARK_STATIC_DEFINE -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -D_QNX_SOURCE -I/home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/include -I/home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/src -fexceptions -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=200112L -D_QNX_SOURCE -fexceptions -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=200112L -D_QNX_SOURCE  -Wall  -Wextra  -Wshadow  -Wfloat-equal  -Wold-style-cast  -Werror  -Wsuggest-override  -pedantic  -pedantic-errors  -fstrict-aliasing  -Wno-deprecated-declarations  -Wno-deprecated  -Wstrict-aliasing -g -std=c++11 -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT src/CMakeFiles/benchmark.dir/sysinfo.cc.o -MF src/CMakeFiles/benchmark.dir/sysinfo.cc.o.d -o src/CMakeFiles/benchmark.dir/sysinfo.cc.o -c /home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/src/sysinfo.cc
/home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/src/sysinfo.cc: In function 'double benchmark::{anonymous}::GetCPUCyclesPerSecond(benchmark::CPUInfo::Scaling)':
/home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/src/sysinfo.cc:774:69: error: use of old-style cast to 'int64_t' {aka 'long int'} [-Werror=old-style-cast]
   return static_cast<double>((int64_t)(SYSPAGE_ENTRY(cpuinfo)->speed) *
                                                                     ^
                              ---------
                              static_cast<int64_t> (                  )
/home/user/vcpkg/buildtrees/benchmark/src/v1.8.3-eb6e61b079.clean/src/sysinfo.cc:775:51: error: use of old-style cast to 'int64_t' {aka 'long int'} [-Werror=old-style-cast]
                              (int64_t)(1000 * 1000));
                                                   ^
                              ---------
                              static_cast<int64_t> ()
cc1plus: all warnings being treated as errors

Additional context

Upstream bug report at google/benchmark#1707

Patch suggestion:

diff --git a/src/sysinfo.cc b/src/sysinfo.cc
index 922e83a..ec9fd35 100644
--- a/src/sysinfo.cc
+++ b/src/sysinfo.cc
@@ -771,8 +771,8 @@ double GetCPUCyclesPerSecond(CPUInfo::Scaling scaling) {
   kstat_close(kc);
   return clock_hz;
 #elif defined(BENCHMARK_OS_QNX)
-  return static_cast<double>((int64_t)(SYSPAGE_ENTRY(cpuinfo)->speed) *
-                             (int64_t)(1000 * 1000));
+  return static_cast<double>(static_cast<int64_t>(SYSPAGE_ENTRY(cpuinfo)->speed) *
+                             static_cast<int64_t>(1000 * 1000));
 #elif defined(BENCHMARK_OS_QURT)
   // QuRT doesn't provide any API to query Hexagon frequency.
   return 1000000000;

This patch fixes the issue and benchmark builds and operates as expected.

[dg0yt]

Needs to get rid of -Werror=old-style-cast.

[Arech]

No. The source code is expected to be built with -Werror. It's fully compatible with it with the only exception of this QNX-specific tiny statement, which was likely just forgotten when -Werror was introduced.

[dg0yt]

The source code is expected to be built with -Werror.

This is a reasonable expectation for controlled environments, and a reasonable default for developers and CI.
It is a fragile assumption for a diverse world (unexpected toolchains) and from a long-term POV (changing environments).

[Arech]

True. But it also gives some guarantees that the code will work as expected (not that important for C-style casts, but is important for many other warnings). I prefer potentially not compilable, but more correct code to a code that compiles, but God knows if it works correctly. Though, I don't have a say in this project.

[BillyONeal]

I think it's google-benchmark's choice if they want to support being built with -Wold-style-cast. I don't think us trying to second guess after the fact and put what casts we think should be there instead makes sense.

I prefer potentially not compilable, but more correct code to a code that compiles, but God knows if it works correctly

If people unfamiliar with the code are making changes, that makes it more likely to be issues, not less.