master/ Fedora 32 / gcc10 FAIL with -Werror=type-limits in google/protobuf/map_lite_unittest.pb.cc

[dimhen]

protobuf
commit 2ae7cf0 (HEAD -> master, origin/master, origin/HEAD)

Fedora 32 / x86_64
gcc version 10.1.1 20200507 (Red Hat 10.1.1-1) (GCC)

git pull
./autogen.sh
./configure --prefix=/usr/local/protobuf
make -j2
V=1 make check
[...]
g++ -DHAVE_CONFIG_H -I. -I.. -pthread -DHAVE_PTHREAD=1 -DHAVE_ZLIB=1 -Wall -Wextra -Werror -Wno-unused-parameter -g -std=c++11 -DNDEBUG -MT google/protobuf/no_warning_test-map_lite_unittest.pb.o -MD -MP -MF google/protobuf/.deps/no_warning_test-map_lite_unittest.pb.Tpo -c -o google/protobuf/no_warning_test-map_lite_unittest.pb.o test -f 'google/protobuf/map_lite_unittest.pb.cc' || echo './'google/protobuf/map_lite_unittest.pb.cc
In file included from ./google/protobuf/map_type_handler.h:34,
from ./google/protobuf/map.h:51,
from ./google/protobuf/generated_message_table_driven.h:34,
from ./google/protobuf/map_lite_unittest.pb.h:26,
from google/protobuf/map_lite_unittest.pb.cc:4:
./google/protobuf/parse_context.h: In instantiation of 'bool google::protobuf::internal::ExpectTag(const char*) [with unsigned int tag = 130]':
google/protobuf/map_lite_unittest.pb.cc:1593:73: required from here
./google/protobuf/parse_context.h:398:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
398 | return ptr == tag;
| ~~~~~^~~~~~
./google/protobuf/parse_context.h: In instantiation of 'bool google::protobuf::internal::ExpectTag(const char) [with unsigned int tag = 138]':
google/protobuf/map_lite_unittest.pb.cc:1605:73: required from here
./google/protobuf/parse_context.h:398:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
./google/protobuf/parse_context.h: In instantiation of 'bool google::protobuf::internal::ExpectTag(const char*) [with unsigned int tag = 146]':
google/protobuf/map_lite_unittest.pb.cc:1617:73: required from here
./google/protobuf/parse_context.h:398:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
./google/protobuf/parse_context.h: In instantiation of 'bool google::protobuf::internal::ExpectTag(const char*) [with unsigned int tag = 810]':
google/protobuf/map_lite_unittest.pb.cc:4074:73: required from here
./google/protobuf/parse_context.h:398:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
./google/protobuf/parse_context.h: In instantiation of 'bool google::protobuf::internal::ExpectTag(const char*) [with unsigned int tag = 818]':
google/protobuf/map_lite_unittest.pb.cc:4087:73: required from here
./google/protobuf/parse_context.h:398:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:4403: google/protobuf/no_warning_test-map_lite_unittest.pb.o] Error 1
make[2]: Leaving directory '/tmp/protobuf/src'
make[1]: *** [Makefile:8191: check-am] Error 2
make[1]: Leaving directory '/tmp/protobuf/src'
make: *** [Makefile:1849: check-recursive] Error 1

[gerben-s]

I feel this is more a "compiler bug" then an issue in our code. gcc is issuing a warning the code actively protects itself from.

[gerben-s]

Here is a small reproduction

https://godbolt.org/z/F9vtCd

If code explicitely checks a case cannot occur then a warning in dead code is IMO a compiler bug in the warnings.

[dimhen]

Thank you for clarification!

FYI: there are already bug report https://gcc.gnu.org/PR95148

[davidlt]

I got this error too on Fedora 33 (Rawhide) on riscv64 while compiling 3.11.4 release.

[..]
./google/protobuf/parse_context.h:397:17: error: comparison is always false due to limited range of data type [-Werror=type-limits]
  397 |     return *ptr == tag;
      |            ~~~~~^~~~~~
[..]
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:4475: google/protobuf/no_warning_test-unittest_lite.pb.o] Error 1
make[2]: *** Waiting for unfinished jobs....
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:4461: google/protobuf/no_warning_test-map_lite_unittest.pb.o] Error 1
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:4573: google/protobuf/no_warning_test-map_proto2_unittest.pb.o] Error 1
make[2]: Leaving directory '/builddir/build/BUILD/protobuf-3.11.4/src'

[Romain-Geissler-1A]

Hi,

@gerben-s Technically this warning can be avoided by using compile time "if": if constexpr (tag < 128) {

I checked on Compiler Explorer and that works (ie the warning goes away with recent gcc). if constexpr is C++17 only, and I know Protobuf right now requires C++11, but can we consider using "if constexpr" conditionally when the feature macro __cpp_if_constexpr is defined ? Note that gcc 11 enables C++17 by default, I also expect LLVM to follow on that path soon as well.

Cheers,
Romain

[Romain-Geissler-1A]

Note: this issue is fixed in Protobuf >= 3.15.0 thanks to PR #8092.

[nmaludy]

@Romain-Geissler-1A i can confirm i'm still seeing this error with 3.15.5 on CentOS 8 with gcc 10.2.0

include/google/protobuf/stubs/logging.h:161:48: error: comparison of unsigned expression in ‘>= 0’ is always true [-Werror=type-limits]
  161 | #define GOOGLE_CHECK_GE(A, B) GOOGLE_CHECK((A) >= (B))
      |                                                ^
include/google/protobuf/stubs/logging.h:151:5: note: in definition of macro ‘GOOGLE_LOG_IF’
  151 |   !(CONDITION) ? (void)0 : GOOGLE_LOG(LEVEL)
      |     ^~~~~~~~~
include/google/protobuf/stubs/logging.h:161:31: note: in expansion of macro ‘GOOGLE_CHECK’
  161 | #define GOOGLE_CHECK_GE(A, B) GOOGLE_CHECK((A) >= (B))
      |                               ^~~~~~~~~~~~
include/google/protobuf/stubs/logging.h:201:26: note: in expansion of macro ‘GOOGLE_CHECK_GE’
  201 | #define GOOGLE_DCHECK_GE GOOGLE_CHECK_GE
      |                          ^~~~~~~~~~~~~~~
include/google/protobuf/parse_context.h:351:7: note: in expansion of macro ‘GOOGLE_DCHECK_GE’
  351 |       GOOGLE_DCHECK_GE(chunk_size, static_cast<size_t>(0));

[Romain-Geissler-1A]

@nmaludy This is not this issue, but #8309 which was merged already (I don't know in which version of Protobuf though).

[nmaludy]

@Romain-Geissler-1A Sorry for the confusion! Thanks for the fix!

[elharo]

so this is fixed?