Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: fix compiler warnings #25165

Closed
wants to merge 1 commit into from
Closed

src: fix compiler warnings #25165

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 21, 2018

The warnings in question are:

../src/node.cc:844:13: warning: unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning: unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The warnings in question are:

../src/node.cc:844:13: warning: unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning: unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 21, 2018
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

In case anyone else was wondering, the functions being referred to were moved in #25127.

@Trott
Copy link
Member

Trott commented Dec 23, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 23, 2018
@Trott
Copy link
Member

Trott commented Dec 23, 2018

The quoted error messages in the commit message are longer than 72 characters. What do we normally do in this case? Land it as-is? Wrap the error message even though that's altering quoted output? Move the error messages to a gist or something and link there?

@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 24, 2018

I'm not sure if we have a general policy on that. In this particular case, [-Wunused-function] is not important and can be dropped.

@danbev
Copy link
Contributor

danbev commented Dec 24, 2018

The quoted error messages in the commit message are longer than 72 characters. What do we normally do in this case? Land it as-is?

I usually line break it and try to make sure that the ^ still lines up:

../src/node.cc:844:13: warning:
unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
                ^
../src/node.cc:845:13: warning:
unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);

I'll do that for this as well but let me know if I should be doing something different in the future.

@danbev
Copy link
Contributor

danbev commented Dec 24, 2018

Landed in e7e26b2. 🎄

@danbev danbev closed this Dec 24, 2018
@cjihrig cjihrig deleted the warning branch December 24, 2018 15:43
MylesBorins referenced this pull request Dec 25, 2018
The warnings in question are:

../src/node.cc:844:13: warning:
unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning:
unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);
@MylesBorins
Copy link
Contributor

landing this on v11.x causes there to be an error as the symbol is still being used. Unfortunately as the commit meta data was not included in the commit when it landed on master there will be no way for our current tooling to ignore this commit when auditing master for v11.x

We likely want to revisit this and try and come up with a solution so it doesn't keep showing up as a false positive when auditing

@danbev
Copy link
Contributor

danbev commented Dec 25, 2018

@MylesBorins I really sorry about this 😞 Is there anything I can do to fix this or is it too late?
Would it help if I reverted this and commit with the metadata?

@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 25, 2018 via email

addaleax pushed a commit to addaleax/node that referenced this pull request Jan 14, 2019
The warnings in question are:

../src/node.cc:844:13: warning:
unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning:
unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);

PR-URL: nodejs#25165
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
The warnings in question are:

../src/node.cc:844:13: warning:
unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning:
unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);

PR-URL: #25165
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

Backport-PR-URL: #25496
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
The warnings in question are:

../src/node.cc:844:13: warning:
unused function 'DebugProcess' [-Wunused-function]
static void DebugProcess(const FunctionCallbackInfo<Value>& args);
            ^
../src/node.cc:845:13: warning:
unused function 'DebugEnd' [-Wunused-function]
static void DebugEnd(const FunctionCallbackInfo<Value>& args);

PR-URL: nodejs#25165
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

Backport-PR-URL: nodejs#25496
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs 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++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants