Skip to content

src: fix -Winconsistent-missing-override warning - #30549

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:warning
Nov 20, 2019
Merged

src: fix -Winconsistent-missing-override warning#30549
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:warning

Conversation

@cjihrig

Copy link
Copy Markdown
Contributor

This commit addresses the following warning:

../src/node_api.cc:28:19: warning: 'mark_arraybuffer_as_untransferable'
overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
v8::Maybe mark_arraybuffer_as_untransferable(
^
../src/js_native_api_v8.h:42:27: note: overridden virtual function is here
virtual v8::Maybe mark_arraybuffer_as_untransferable(

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels Nov 19, 2019
@addaleax

Copy link
Copy Markdown
Member

Feel free to approve fast-tracking by 👍ing this comment

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Nov 19, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

nodejs-github-bot commented Nov 20, 2019

Copy link
Copy Markdown
Collaborator

This commit addresses the following warning:
../src/node_api.cc:28:19: warning: 'mark_arraybuffer_as_untransferable'
overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
v8::Maybe<bool> mark_arraybuffer_as_untransferable(
^
../src/js_native_api_v8.h:42:27: note: overridden virtual function is here
virtual v8::Maybe<bool> mark_arraybuffer_as_untransferable(
PR-URL: nodejs#30549
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@cjihrig
cjihrig merged commit 3871cc5 into nodejs:masterNov 20, 2019
@cjihrig
cjihrig deleted the warning branch November 20, 2019 18:53
MylesBorins pushed a commit that referenced this pull request Nov 21, 2019
This commit addresses the following warning:
../src/node_api.cc:28:19: warning: 'mark_arraybuffer_as_untransferable'
overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
v8::Maybe<bool> mark_arraybuffer_as_untransferable(
^
../src/js_native_api_v8.h:42:27: note: overridden virtual function is here
virtual v8::Maybe<bool> mark_arraybuffer_as_untransferable(
PR-URL: #30549
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@BridgeARBridgeAR mentioned this pull request Nov 21, 2019
@codebytere

Copy link
Copy Markdown
Member

@targos why can't this be backported to 12.x? it's causing compilation errors in Electron at the moment, so i'd like to get it there if possible.

targos pushed a commit that referenced this pull request Feb 13, 2020
This commit addresses the following warning:
../src/node_api.cc:28:19: warning: 'mark_arraybuffer_as_untransferable'
overrides a member function but is not marked 'override'
[-Winconsistent-missing-override]
v8::Maybe<bool> mark_arraybuffer_as_untransferable(
^
../src/js_native_api_v8.h:42:27: note: overridden virtual function is here
virtual v8::Maybe<bool> mark_arraybuffer_as_untransferable(
PR-URL: #30549
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
@targos

Copy link
Copy Markdown
Member

I pushed it to staging. I initially thought the commit that introduced the warning wouldn't be backported to v12.x.

@MylesBorinsMylesBorins mentioned this pull request Feb 13, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 18, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
* chore: bump node in DEPS to v12.16.0
* Fixup asar support setup patch
nodejs/node#30862
* Fixup InternalCallbackScope patch
nodejs/node#30236
* Fixup GN buildfiles patch
nodejs/node#30755
* Fixup low-level hooks patch
nodejs/node#30466
* Fixup globals require patch
nodejs/node#31643
* Fixup process stream patch
nodejs/node#30862
* Fixup js2c modification patch
nodejs/node#30755
* Fixup internal fs override patch
nodejs/node#30610
* Fixup context-aware warn patch
nodejs/node#30336
* Fixup Node.js with ltcg config
nodejs/node#29388
* Fixup oaepLabel patch
nodejs/node#30917
* Remove redundant ESM test patch
nodejs/node#30997
* Remove redundant cli flag patch
nodejs/node#30466
* Update filenames.json
* Remove macro generation in GN build files
nodejs/node#30755
* Fix some compilation errors upstream
* Add uvwasi to deps
nodejs/node#30258
* Fix BoringSSL incompatibilities
* Fixup linked module patch
nodejs/node#30274
* Add missing sources to GN uv build
libuv/libuv#2347
* Patch some uvwasi incompatibilities
* chore: bump Node.js to v12.6.1
* Remove mark_arraybuffer_as_untransferable.patch
nodejs/node#30549
* Fix uvwasi build failure on win
* Fixup --perf-prof cli option error
* Fixup early cjs module loading
* fix: initialize diagnostics properly
nodejs/node#30025
* Disable new esm syntax specs
nodejs/node#30219
* Fixup v8 weakref hook spec
nodejs/node#29874
* Fix async context timer issue
* Disable monkey-patch-main spec
It relies on nodejs/node#29777, and we don't
override prepareStackTrace.
* Disable new tls specs
nodejs/node#23188
We don't support much of TLS owing to schisms between BoringSSL and
OpenSSL.
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.fast-trackPRs that do not need to wait for 48 hours to land.node-apiIssues and PRs related to the Node-API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@cjihrig@addaleax@nodejs-github-bot@codebytere@targos@bnoordhuis@gengjiawen@devnexen@BridgeAR