FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: fix compiler warnings in node_http2.cc by danbev · Pull Request #33014 · nodejs/node · GitHub

/ node Public

src: fix compiler warnings in node_http2.cc - #33014

Closed
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:http2-compiler-warnings
ConversationCommits (1)ChecksFiles changed
Closed

src: fix compiler warnings in node_http2.cc#33014
danbev wants to merge 1 commit into
nodejs:masterfrom
danbev:http2-compiler-warnings

Conversation

danbev commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

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

src: fix compiler warnings in node_http2.cc
7046c59 
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 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

nodejs-github-bot commented Apr 23, 2020

Copy link
Copy Markdown
Collaborator

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

jasnell approved these changes Apr 23, 2020
View reviewed changes

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Apr 23, 2020

Copy link
Copy Markdown
Member

Fast track?

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

nodejs-github-bot commented Apr 23, 2020

Copy link
Copy Markdown
Collaborator

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

jasnell commented Apr 23, 2020

Copy link
Copy Markdown
Member

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

nodejs-github-bot commented Apr 23, 2020

Copy link
Copy Markdown
Collaborator

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

danbev commented Apr 24, 2020

Copy link
Copy Markdown
Contributor Author

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 commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

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

richardlau mentioned this pull request Apr 24, 2020
Closed
2 tasks
addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

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

jasnell pushed a commit that referenced this pull request Apr 24, 2020
src: fix compiler warnings in node_http2.cc
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 <himself65@outlook.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

jasnell commented Apr 24, 2020

Copy link
Copy Markdown
Member

Landed in 654c0ac

jasnell closed this Apr 24, 2020
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
src: fix compiler warnings in node_http2.cc
14f8864 
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 <himself65@outlook.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
BethGriggs mentioned this pull request Apr 27, 2020
Merged

BridgeAR commented Apr 28, 2020

Copy link
Copy Markdown
Member

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?

BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
src: fix compiler warnings in node_http2.cc
bc755fc 
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 <himself65@outlook.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
joyeecheung mentioned this pull request Jun 8, 2020
Closed
3 tasks
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes
himself65 himself65 approved these changes
juanarbol juanarbol approved these changes
+1 more reviewer
sam-github sam-github approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. http2 Issues or PRs related to the http2 subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL