Skip to content

tools: fix inspector_protocol compiler warnings

Rodrigo Muino Tomonari requested to merge github/fork/richardlau/inspector into master

This is an attempt to see if cherry-picking https://github.com/nodejs/node/commit/ffb34b6d5dbf002f182f08e45c32883d7174be8b (reverted by https://github.com/nodejs/node/pull/39694) fixes the currently broken coverage workflow.

https://github.com/nodejs/node/actions/workflows/coverage-linux.yml?query=

image

/home/runner/work/node/node/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp: In member function ‘void node::inspector::protocol::cbor::CBORTokenizer::ReadNextToken(bool)’:
/home/runner/work/node/node/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:2567:66: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint64_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
 2567 |           if (!bytes_read || std::numeric_limits<int32_t>::max() <
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
 2568 |                                  token_start_internal_value_) {
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~      
/home/runner/work/node/node/out/Release/obj/gen/src/node/inspector/protocol/Protocol.cpp:2585:58: error: comparison of integer expressions of different signedness: ‘uint64_t’ {aka ‘long unsigned int’} and ‘int’ [-Werror=sign-compare]
 2585 |           if (!bytes_read || token_start_internal_value_ >
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
 2586 |                                  std::numeric_limits<int32_t>::max()) {
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [libnode.target.mk:410: /home/runner/work/node/node/out/Release/obj.target/libnode/gen/src/node/inspector/protocol/Protocol.o] Error 1

The second commit reenables coverage (the workflow that appears to be broken) for tools as that's where the inspector protocol files are (I forget the reason but it was moved there a long time ago) and the workflow wasn't run on https://github.com/nodejs/node/pull/39694.

Merge request reports