src: fix compiler warnings in node_http2.cc #33014

danbev · 2020-04-23T04:18:12Z

Currently, the following compiler warnings are generated:

../src/node_http2.cc: In static member function
‘static int node::http2::Http2Session::OnStreamClose(nghttp2_session*,
    int32_t, uint32_t, void*)’:
../src/node_http2.cc:994:16: warning:
variable ‘def’ set but not used [-Wunused-but-set-variable]
  994 |   Local<Value> def = v8::False(env->isolate());
      |                ^~~
../src/node_http2.cc: In static member function
‘static void node::http2::Http2Session::Ping(
    const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/node_http2.cc:2755:16: warning:
unused variable ‘env’ [-Wunused-variable]
 2755 |   Environment* env = Environment::GetCurrent(args);
      |                ^~~
../src/node_http2.cc: In static member function
‘static void node::http2::Http2Session::Settings(
const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/node_http2.cc:2774:16: warning:
unused variable ‘env’ [-Wunused-variable]
 2774 |   Environment* env = Environment::GetCurrent(args);
      |                ^~~

This commit removes these unused variables.

Checklist

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

Currently, the following compiler warnings are generated: ../src/node_http2.cc: In static member function ‘static int node::http2::Http2Session::OnStreamClose(nghttp2_session*, int32_t, uint32_t, void*)’: ../src/node_http2.cc:994:16: warning: variable ‘def’ set but not used [-Wunused-but-set-variable] 994 | Local<Value> def = v8::False(env->isolate()); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Ping( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2755:16: warning: unused variable ‘env’ [-Wunused-variable] 2755 | Environment* env = Environment::GetCurrent(args); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Settings( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2774:16: warning: unused variable ‘env’ [-Wunused-variable] 2774 | Environment* env = Environment::GetCurrent(args); | ^~~ This commit removes these unused variables.

nodejs-github-bot · 2020-04-23T05:13:23Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30917/

jasnell

Ugh, thank you... so annoying that the Windows build does not give the same warnings. It would definitely be good to get that PR elevating warnings into failures in CI landed

jasnell · 2020-04-23T16:23:04Z

Fast track?

nodejs-github-bot · 2020-04-23T18:26:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30934/

jasnell · 2020-04-23T18:26:59Z

CI hung on arm-fanned tho no errors were reported. running again

nodejs-github-bot · 2020-04-23T19:05:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30936/

danbev · 2020-04-24T07:16:05Z

It would definitely be good to get that PR elevating warnings into failures in CI landed

Sorry, I dropped the ball on this one. I've updated #32685 now and hopefully we can get it landed. We also need to add this to the CI jobs before it takes effect on all PRs.

nodejs-github-bot · 2020-04-24T07:18:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30943/

nodejs-github-bot · 2020-04-24T17:05:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30954/

Currently, the following compiler warnings are generated: ../src/node_http2.cc: In static member function ‘static int node::http2::Http2Session::OnStreamClose(nghttp2_session*, int32_t, uint32_t, void*)’: ../src/node_http2.cc:994:16: warning: variable ‘def’ set but not used [-Wunused-but-set-variable] 994 | Local<Value> def = v8::False(env->isolate()); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Ping( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2755:16: warning: unused variable ‘env’ [-Wunused-variable] 2755 | Environment* env = Environment::GetCurrent(args); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Settings( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2774:16: warning: unused variable ‘env’ [-Wunused-variable] 2774 | Environment* env = Environment::GetCurrent(args); | ^~~ This commit removes these unused variables. PR-URL: #33014 Reviewed-By: Zeyu Yang <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]>

jasnell · 2020-04-24T18:07:59Z

Landed in 654c0ac

Currently, the following compiler warnings are generated: ../src/node_http2.cc: In static member function ‘static int node::http2::Http2Session::OnStreamClose(nghttp2_session*, int32_t, uint32_t, void*)’: ../src/node_http2.cc:994:16: warning: variable ‘def’ set but not used [-Wunused-but-set-variable] 994 | Local<Value> def = v8::False(env->isolate()); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Ping( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2755:16: warning: unused variable ‘env’ [-Wunused-variable] 2755 | Environment* env = Environment::GetCurrent(args); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Settings( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2774:16: warning: unused variable ‘env’ [-Wunused-variable] 2774 | Environment* env = Environment::GetCurrent(args); | ^~~ This commit removes these unused variables. PR-URL: #33014 Reviewed-By: Zeyu Yang <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]>

BridgeAR · 2020-04-28T10:35:23Z

This lands cleanly on v13.x but it's required and should not land therefore or another commit should land first. It should probably not be backported at all?

Currently, the following compiler warnings are generated: ../src/node_http2.cc: In static member function ‘static int node::http2::Http2Session::OnStreamClose(nghttp2_session*, int32_t, uint32_t, void*)’: ../src/node_http2.cc:994:16: warning: variable ‘def’ set but not used [-Wunused-but-set-variable] 994 | Local<Value> def = v8::False(env->isolate()); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Ping( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2755:16: warning: unused variable ‘env’ [-Wunused-variable] 2755 | Environment* env = Environment::GetCurrent(args); | ^~~ ../src/node_http2.cc: In static member function ‘static void node::http2::Http2Session::Settings( const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_http2.cc:2774:16: warning: unused variable ‘env’ [-Wunused-variable] 2774 | Environment* env = Environment::GetCurrent(args); | ^~~ This commit removes these unused variables. PR-URL: #33014 Reviewed-By: Zeyu Yang <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. labels Apr 23, 2020

himself65 approved these changes Apr 23, 2020

View reviewed changes

richardlau approved these changes Apr 23, 2020

View reviewed changes

juanarbol approved these changes Apr 23, 2020

View reviewed changes

cjihrig approved these changes Apr 23, 2020

View reviewed changes

jasnell approved these changes Apr 23, 2020

View reviewed changes

jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 23, 2020

sam-github approved these changes Apr 23, 2020

View reviewed changes

richardlau mentioned this pull request Apr 24, 2020

build: turn warnings into errors for node sources #32685

Closed

2 tasks

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 24, 2020

jasnell closed this Apr 24, 2020

BethGriggs mentioned this pull request Apr 27, 2020

v14.1.0 proposal #33103

Merged

BridgeAR added the dont-land-on-v13.x label Apr 28, 2020

targos added the backport-blocked-v12.x label Apr 29, 2020

joyeecheung mentioned this pull request Jun 8, 2020

src: retrieve binding data from the context #33139

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: fix compiler warnings in node_http2.cc #33014

src: fix compiler warnings in node_http2.cc #33014

danbev commented Apr 23, 2020

nodejs-github-bot commented Apr 23, 2020

jasnell left a comment

jasnell commented Apr 23, 2020

nodejs-github-bot commented Apr 23, 2020

jasnell commented Apr 23, 2020

nodejs-github-bot commented Apr 23, 2020

danbev commented Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

jasnell commented Apr 24, 2020

BridgeAR commented Apr 28, 2020

src: fix compiler warnings in node_http2.cc #33014

src: fix compiler warnings in node_http2.cc #33014

Conversation

danbev commented Apr 23, 2020

Checklist

nodejs-github-bot commented Apr 23, 2020

jasnell left a comment

Choose a reason for hiding this comment

jasnell commented Apr 23, 2020

nodejs-github-bot commented Apr 23, 2020

jasnell commented Apr 23, 2020

nodejs-github-bot commented Apr 23, 2020

danbev commented Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

jasnell commented Apr 24, 2020

BridgeAR commented Apr 28, 2020