GCC warnings when compiling files in src

[targos]

I don't know if they are all fixable, but the first one is very annoying because it is emitted for (almost?) each C++ file:

In file included from ../src/node.h:63,
                 from ../src/api/callback.cc:1:
../deps/v8/include/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::BaseObject; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)]’:
../src/base_object-inl.h:104:42:   required from here
../deps/v8/include/v8.h:9585:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::BaseObject>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
                reinterpret_cast<Callback>(callback), type);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In file included from ../src/node_buffer.cc:29:
../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = short unsigned int]’:
../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     return (this->*strategy_)(subject, index);
            ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = unsigned char]’:
../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     return (this->*strategy_)(subject, index);
            ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

[refack]

https://gist.github.com/refack/1cc202e9b507d63d108d5ba55293e540 with 167 warnings from MSVC

[cjihrig]

@targos is this still an issue?

[targos]

@cjihrig Yes, I can still reproduce all warnings from the OP with GCC 8 and 9.

[bmsdave]

@targos I would like to try to solve this problem. I'll try it next week.

[bmsdave]

@targos hi. Can you describe the steps to reproduce these notifications?

[targos]

@bmsdave Hello. I can reproduce the warnings on my system (Fedora 30, GCC 9.2.1) with:

mkdir -p out/Release
cd out/Release

$ c++ -MMD -MF obj/src/libnode.node.o.d -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS '-DNODE_ARCH="x64"' '-DNODE_PLATFORM="linux"' -DNODE_WANT_INTERNALS=1 -DV8_DEPRECATION_WARNINGS=1 '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' -DHAVE_INSPECTOR=1 -DNODE_REPORT -D__POSIX__ -DNODE_USE_V8_PLATFORM=1 -DNODE_HAVE_I18N_SUPPORT=1 -DHAVE_OPENSSL=1 -DUCONFIG_NO_SERVICE=1 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION=1 -DU_HAVE_STD_STRING=1 -DUCONFIG_NO_BREAK_ITERATION=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_POSIX_C_SOURCE=200112 -DNGHTTP2_STATICLIB -I../../src -Igen -Igen/include -Igen/src -I../../deps/histogram/src -I../../deps/v8/include -I../../deps/icu-small/source/i18n -I../../deps/icu-small/source/common -I../../deps/zlib -I../../deps/llhttp/include -I../../deps/cares/include -I../../deps/uv/include -I../../deps/nghttp2/lib/includes -I../../deps/brotli/c/include -I../../deps/openssl/openssl/include -Wall -Wextra -Wno-unused-parameter -pthread -Wall -Wextra -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++1y  -c ../../src/node.cc -o obj/src/libnode.node.o
In file included from ../../src/node.h:63,
                 from ../../src/node.cc:22:
../../deps/v8/include/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::BaseObject; T = v8::Object; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)]’:
../../src/base_object-inl.h:106:42:   required from here
../../deps/v8/include/v8.h:10100:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::BaseObject>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::BaseObject>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
10100 |                reinterpret_cast<Callback>(callback), type);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

$ c++ -MMD -MF obj/src/libnode.node_buffer.o.d -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS '-DNODE_ARCH="x64"' '-DNODE_PLATFORM="linux"' -DNODE_WANT_INTERNALS=1 -DV8_DEPRECATION_WARNINGS=1 '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' -DHAVE_INSPECTOR=1 -DNODE_REPORT -D__POSIX__ -DNODE_USE_V8_PLATFORM=1 -DNODE_HAVE_I18N_SUPPORT=1 -DHAVE_OPENSSL=1 -DUCONFIG_NO_SERVICE=1 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION=1 -DU_HAVE_STD_STRING=1 -DUCONFIG_NO_BREAK_ITERATION=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_POSIX_C_SOURCE=200112 -DNGHTTP2_STATICLIB -I../../src -Igen -Igen/include -Igen/src -I../../deps/histogram/src -I../../deps/v8/include -I../../deps/icu-small/source/i18n -I../../deps/icu-small/source/common -I../../deps/zlib -I../../deps/llhttp/include -I../../deps/cares/include -I../../deps/uv/include -I../../deps/nghttp2/lib/includes -I../../deps/brotli/c/include -I../../deps/openssl/openssl/include -Wall -Wextra -Wno-unused-parameter -pthread -Wall -Wextra -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++1y  -c ../../src/node_buffer.cc -o obj/src/libnode.node_buffer.o
In file included from ../../src/node.h:63,
                 from ../../src/node_buffer.h:25,
                 from ../../src/node_buffer.cc:22:
../../deps/v8/include/v8.h: In instantiation of ‘void v8::PersistentBase<T>::SetWeak(P*, typename v8::WeakCallbackInfo<P>::Callback, v8::WeakCallbackType) [with P = node::Buffer::{anonymous}::CallbackInfo; T = v8::ArrayBuffer; typename v8::WeakCallbackInfo<P>::Callback = void (*)(const v8::WeakCallbackInfo<node::Buffer::{anonymous}::CallbackInfo>&)]’:
../../src/node_buffer.cc:130:75:   required from here
../../deps/v8/include/v8.h:10100:16: warning: cast between incompatible function types from ‘v8::WeakCallbackInfo<node::Buffer::{anonymous}::CallbackInfo>::Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<node::Buffer::{anonymous}::CallbackInfo>&)’} to ‘Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo<void>&)’} [-Wcast-function-type]
10100 |                reinterpret_cast<Callback>(callback), type);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../src/node_buffer.cc:29:
../../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = short unsigned int]’:
../../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  113 |     return (this->*strategy_)(subject, index);
      |            ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
../../src/string_search.h: In function ‘size_t node::stringsearch::SearchString(node::stringsearch::Vector<const Char>, node::stringsearch::Vector<const Char>, size_t) [with Char = unsigned char]’:
../../src/string_search.h:113:30: warning: ‘search’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  113 |     return (this->*strategy_)(subject, index);
      |            ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

I removed the other two warnings from the OP because I can't reproduce them anymore.

[gireeshpunathil]

just wondering between this and https://github.com/nodejs/node/issues/18983 , are we talking about the same thing?

[targos]

It doesn't look like the same thing.

[lundibundi]

Not sure we can do anything about the rest of them:

The search_string ones are caused by uninitialized arrays (they are initialized when needed and not upon object creation) in SearchStringBase which can be initialized (i.e. = {0}) with 1-2% performance impact or suppressed with something like #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" (this should also work for clang). Not sure if this is needed though pragma ignore would probably be fine.

The Wcast-function-type is from the base_object-inl.h specifically the v8::PersistentBase::SetWeak call which uses reinterpret_cast from WeakCallbackInfo<Type> to WeakCallbackInfo<void> which is okay for C++ and is just fine in there IIUC. I don't think this is possible to fix on our end. I'd consider this one as wontfix however annoying it is, unfortunately.