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.

Sign up for GitHub

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

Jump to bottom

src: ignore unused warning for inspector-agent.cc #13188

Closed
wants to merge 1 commit into from
Closed

src: ignore unused warning for inspector-agent.cc #13188

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/inspector_agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class JsBindingsSessionDelegate : public InspectorSessionDelegate {
Local<Value> argument = v8string.ToLocalChecked().As<Value>();
Local<Function> callback = callback_.Get(isolate);
Local<Object> receiver = receiver_.Get(isolate);
callback->Call(env_->context(), receiver, 1, &argument);
static_cast<void>(callback->Call(env_->context(), receiver, 1, &argument));
Copy link
Contributor

@refack refack May 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't

#pragma GCC unused-result push
#pragma GCC unused-result ignored
callback->Call(env_->context(), receiver, 1, &argument)
#pragma GCC unused-result pop

be better, so it could be traced in the future

Or even just a comment stating the the // static_cast<void> is to skip 'unused-result' warning?

Dismissed

Copy link
Contributor

Choose a reason for hiding this comment

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

No comment on the portability of the pragma, but there is no reason to cast to void other than to avoid warnings and visually document that the return value is explicitly being ignored, I don't think a comment is needed for that usage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed if we document this somewhere as an idiom static_cast<void> === #pragma GCC unused-result ignored
Because a quick search only found one other use, in this file in line 210@master 352@here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we don't have a style guide, I'll add a note to #12791

Copy link
Contributor

Choose a reason for hiding this comment

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

Casting to void is standard C++, https://stackoverflow.com/questions/689677/why-cast-unused-return-values-to-void, unlike GCC pramas.

Copy link
Contributor

Choose a reason for hiding this comment

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

I already agreed 👌

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I thought you wanted this documented as a node-specific style. Nice emoji use.

}

void Disconnect() {
Expand Down