src: prefer C++ empty() in boolean expressions#34432
src: prefer C++ empty() in boolean expressions#34432tniessen wants to merge 1 commit intonodejs:masterfrom
Conversation
|
Maybe also add |
src/node_url.cc
Outdated
There was a problem hiding this comment.
Wildly inefficient way of removing empty prefixes... (n reallocations + n*m copies when 1 + m suffices)
| 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 |
There was a problem hiding this comment.
@bnoordhuis In your suggestion, the from == begin part is always true, unless i’m missing something?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
As this is not directly related to the other changes in this PR, this could be done in a separate commit/PR.
|
Failed test case in macOS. is this a flaky test ? cc @Trott Path: parallel/test-macos-app-sandbox
--- stderr ---
assert.js:103
throw new AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
1 !== 0
at Object.<anonymous> (/Users/runner/work/node/node/test/parallel/test-macos-app-sandbox.js:41:8)
at Module._compile (internal/modules/cjs/loader.js:1252:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1273:10)
at Module.load (internal/modules/cjs/loader.js:1101:32)
at Function.Module._load (internal/modules/cjs/loader.js:966:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
at internal/main/run_main_module.js:17:47 {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: 1,
expected: 0,
operator: 'strictEqual'
} |
36b41e6 to
11d860c
Compare
|
Landed in 1be7bbd |
PR-URL: #34432 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: #34432 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: #34432 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: #34432 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: #34432 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
This changes occurrences of
length()andsize()toempty()where it makes the code simpler.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes