Skip to content

Conversation

@addaleax
Copy link
Member

Since for all affected conversions there is a preceding type check,
it’s safe to use .FromJust() instead.

This fixes the following compile warnings:

../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ 
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^
@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Feb 7, 2020
@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member

Ugh so irritating these aren't reported when building on Windows.

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 9, 2020
addaleax added a commit that referenced this pull request Feb 10, 2020
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ PR-URL: #31689 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@addaleax
Copy link
MemberAuthor

Landed in 2b1c18f

@addaleaxaddaleax deleted the url-compile-warnings branch February 10, 2020 14:43
codebytere pushed a commit that referenced this pull request Feb 17, 2020
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ PR-URL: #31689 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 15, 2020
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ PR-URL: #31689 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ PR-URL: #31689 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@codebyterecodebytere mentioned this pull request Mar 17, 2020
codebytere pushed a commit that referenced this pull request Mar 30, 2020
Since for all affected conversions there is a preceding type check, it’s safe to use `.FromJust()` instead. This fixes the following compile warnings: ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2242:43: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = long int]’, declared with attribute warn_unused_result [-Wunused-result] 2242 | args[1]->IntegerValue(env->context()).To(&start); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~ ../src/node_url.cc: In function ‘node::url::url_data node::url::{anonymous}::HarvestBase(node::Environment*, v8::Local<v8::Object>)’: ../src/node_url.cc:1206:34: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1206 | flags->Int32Value(context).To(&base.flags); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ ../src/node_url.cc:1211:33: warning: ignoring return value of ‘bool v8::Maybe<T>::To(T*) const [with T = int]’, declared with attribute warn_unused_result [-Wunused-result] 1211 | port->Int32Value(context).To(&base.port); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~ ../src/node_url.cc: In function ‘void node::url::{anonymous}::ToUSVString(const v8::FunctionCallbackInfo<v8::Value>&)’: ../src/node_url.cc:2245:15: warning: ‘start’ may be used uninitialized in this function [-Wmaybe-uninitialized] 2245 | for (size_t i = start; i < value.length(); i++){| ^ PR-URL: #31689 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@addaleax@nodejs-github-bot@jasnell@targos@cjihrig@devnexen@BridgeAR