search

emilk / loguru

A lightweight C++ logging library
The Unlicense
1.76k stars 256 forks source link

compilation fail on linux aarch64 ppc64le and s390x because of -Werror=type-limits #249

Open ibotty opened 6 months ago

ibotty commented 6 months ago

See this example log

/usr/bin/g++ -Dloguru_EXPORTS -I/builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1 -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -mbranch-protection=standard -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -DNDEBUG -fPIC -Wall -Wextra -Werror -pedantic -MD -MT CMakeFiles/loguru.dir/loguru.cpp.o -MF CMakeFiles/loguru.dir/loguru.cpp.o.d -o CMakeFiles/loguru.dir/loguru.cpp.o -c /builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1/loguru.cpp
/builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1/loguru.cpp: In function ‘void loguru::escape(std::string&, const std::string&)’:
/builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1/loguru.cpp:571:36: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  571 |                         else if (0 <= c && c < 0x20) { // ASCI control character:
      |                                  ~~^~~~
/builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1/loguru.cpp: In function ‘loguru::Text loguru::ec_to_text(char)’:
/builddir/build/BUILD/loguru-4adaa185883e3c04da25913579c451d3c32cfac1/loguru.cpp:1845:28: error: comparison is always true due to limited range of data type [-Werror=type-limits]
 1845 |                 else if (0 <= c && c < 0x20) {
      |                          ~~^~~~
cc1plus: all warnings being treated as errors
virtuosonic commented 3 months ago

i got the same error under GCC arm-linux-gnueabihf

virtuosonic commented 3 months ago

https://github.com/virtuosonic/loguru/ fixes this

musicinmybrain commented 3 months ago

In the new loguru package for Fedora, I’m dealing with this by just removing -Werror, which I think is too strict for downstream packaging in general:

sed -r -i 's/;?-Werror\b//' CMakeLists.txt test/CMakeLists.txt

It’s pretty clear that the warning, while technically correct, is not indicating a real problem here.

The patch https://github.com/virtuosonic/loguru/commit/e1ffdc4149083cc221d44b666a0f7e3ec4a87259 from the fork linked above does look, at a glance, like a correct way to avoid the warning without changing the code’s behavior across architectures and compilers.

virtuosonic commented 3 months ago

The patch https://github.com/virtuosonic/loguru/commit/e1ffdc4149083cc221d44b666a0f7e3ec4a87259 from the fork linked above does look, at a glance, like a correct way to avoid the warning without changing the code’s behavior across architectures and compilers.

Actually there no risk if the variable c in the loops is signed or unsigned because it isn't modified, my next step would be to make it const, to make this clear to the compiler.