[v13.x backport] src: ignore GCC -Wcast-function-type for v8.h#31649
Closed
kasicka wants to merge 2 commits intonodejs:v13.x-stagingfrom
kasicka:backport-31475-31524-13.x
Closed
[v13.x backport] src: ignore GCC -Wcast-function-type for v8.h#31649kasicka wants to merge 2 commits intonodejs:v13.x-stagingfrom kasicka:backport-31475-31524-13.x
kasicka wants to merge 2 commits intonodejs:v13.x-stagingfrom
kasicka:backport-31475-31524-13.x
Conversation
This commit suggests that cast-function-type warnings be ignored
from v8.h. Currently, GCC reports a number of warnings like this:
In file included from ../src/util.h:27,
from ../src/aliased_buffer.h:7,
from ../src/memory_tracker.h:5,
from ../src/base_object.h:27,
from ../src/async_wrap.h:27,
from ../src/req_wrap.h:6,
from ../src/req_wrap-inl.h:6,
from ../src/connect_wrap.h:6,
from ../src/connect_wrap.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:123:42: required from here
../deps/v8/include/v8.h:10374: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);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The motivation for doing this that it makes it difficult to spot
other warnings that might be important. Since it is v8 that
performs this cast I was not able to find a way around it.
PR-URL: #31475
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Member
|
Thanks! These seem to land without conflicts on v13.x-staging, so I don’t think a separate backport PR is necessary here. I’ve pushed the commits from master to v13.x-staging. |
Author
|
Was a PR needed for v12? |
Member
|
@kasicka If you didn’t run into any merge conflicts, probably not. Usually, PRs get picked up into LTS releases like v12.x after they have been in a Current release for two weeks. It can be nice to comment in the original PR(s) in the cases in which you do need a backport to v12.x, though, just to let people know that it’s important to you and sometimes to skip that waiting period – opening explicit backport PRs basically does that, too. |
Author
|
Okay, thanks for clarifying. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Backport of #31475 and #31524
Builds fine with GCC 9.2.1 and 7.3.1
make -j4 test(UNIX), orvcbuild test(Windows) passes