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

src: prefer C++ empty() in boolean expressions #34432

Closed
wants to merge 1 commit into from
Closed

src: prefer C++ empty() in boolean expressions #34432

Show file tree
Hide file tree
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 @@ -366,7 +366,7 @@ bool IsFilePath(const std::string& path) {
}
#else
bool IsFilePath(const std::string& path) {
return path.length() && path[0] == '/';
return !path.empty() && path[0] == '/';
}
#endif // __POSIX__

Expand Down
2 changes: 1 addition & 1 deletion src/inspector_js_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ void WaitForDebugger(const FunctionCallbackInfo<Value>& args) {
void Url(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
std::string url = env->inspector_agent()->GetWsUrl();
if (url.length() == 0) {
if (url.empty()) {
return;
}
args.GetReturnValue().Set(OneByteString(env->isolate(), url.c_str()));
Expand Down
2 changes: 1 addition & 1 deletion src/node_file-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void FSContinuationData::MaybeSetFirstPath(const std::string& path) {
}

std::string FSContinuationData::PopPath() {
CHECK_GT(paths_.size(), 0);
CHECK(!paths_.empty());
std::string path = std::move(paths_.back());
paths_.pop_back();
return path;
Expand Down
8 changes: 4 additions & 4 deletions src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,7 +1517,7 @@ void Http2Session::ClearOutgoing(int status) {

set_sending(false);

if (outgoing_buffers_.size() > 0) {
if (!outgoing_buffers_.empty()) {
outgoing_storage_.clear();
outgoing_length_ = 0;

Expand All @@ -1536,7 +1536,7 @@ void Http2Session::ClearOutgoing(int status) {

// Now that we've finished sending queued data, if there are any pending
// RstStreams we should try sending again and then flush them one by one.
if (pending_rst_streams_.size() > 0) {
if (!pending_rst_streams_.empty()) {
std::vector<int32_t> current_pending_rst_streams;
pending_rst_streams_.swap(current_pending_rst_streams);

Expand Down Expand Up @@ -1595,8 +1595,8 @@ uint8_t Http2Session::SendPendingData() {
ssize_t src_length;
const uint8_t* src;

CHECK_EQ(outgoing_buffers_.size(), 0);
CHECK_EQ(outgoing_storage_.size(), 0);
CHECK(outgoing_buffers_.empty());
CHECK(outgoing_storage_.empty());

// Part One: Gather data from nghttp2

Expand Down
8 changes: 4 additions & 4 deletions src/node_url.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@ bool ParseHost(const std::string& input,
std::string* output,
bool is_special,
bool unicode = false) {
if (input.length() == 0) {
if (input.empty()) {
output->clear();
return true;
}
Expand Down Expand Up @@ -2037,7 +2037,7 @@ void URL::Parse(const char* input,
(ch == kEOL ||
ch == '?' ||
ch == '#')) {
while (url->path.size() > 1 && url->path[0].length() == 0) {
while (url->path.size() > 1 && url->path[0].empty()) {
url->path.erase(url->path.begin());
}
Comment on lines +2040 to 2042
Copy link
Member

Choose a reason for hiding this comment

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

Wildly inefficient way of removing empty prefixes... (n reallocations + n*m copies when 1 + m suffices)

Suggested change
while (url->path.size() > 1 && url->path[0].empty()) {
url->path.erase(url->path.begin());
}
auto begin = url->path.begin();
auto end = url->path.end();
auto from = begin;
auto to = begin;
while (to != end && to->empty()) ++to;
if (to == end && from == begin && from != to) ++from;
url->path.erase(from, to); // 1 + m

Copy link
Member

Choose a reason for hiding this comment

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

@bnoordhuis In your suggestion, the from == begin part is always true, unless i’m missing something?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, yeah. It was intended as a kind of self-documentation about the preconditions but I guess it's confusing more than anything else. if (to == end && from != end) ++from; is probably a clearer way of writing it.

Copy link
Member

Choose a reason for hiding this comment

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

As this is not directly related to the other changes in this PR, this could be done in a separate commit/PR.

}
Expand All @@ -2060,9 +2060,9 @@ void URL::Parse(const char* input,
state = kFragment;
break;
default:
if (url->path.size() == 0)
if (url->path.empty())
url->path.emplace_back("");
if (url->path.size() > 0 && ch != kEOL)
else if (ch != kEOL)
AppendOrEscape(&url->path[0], ch, C0_CONTROL_ENCODE_SET);
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/node_worker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
per_isolate_opts.get(),
kAllowedInEnvironment,
&errors);
if (errors.size() > 0 && args[1]->IsObject()) {
if (!errors.empty() && args[1]->IsObject()) {
// Only fail for explicitly provided env, this protects from failures
// when NODE_OPTIONS from parent's env is used (which is the default).
Local<Value> error;
Expand Down