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: remove return after abort #46172

Merged
merged 1 commit into from
Jan 13, 2023
Merged

src: remove return after abort #46172

merged 1 commit into from
Jan 13, 2023

Conversation

codebytere
Copy link
Member

@codebytere codebytere commented Jan 11, 2023 •
edited
Loading

Refs electron/electron#36818.

This extra return causes a build failure in Electron:

../../third_party/electron_node/src/node_builtins.cc:250:5: error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
    return;
    ^~~~~~
1 error generated.

@codebytere codebytere requested a review from mhdawson January 11, 2023 11:20
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jan 11, 2023
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 11, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 11, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/48936/

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 11, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/48944/

@github-actions github-actions bot mentioned this pull request Jan 12, 2023
Open
26 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/48951/

@github-actions github-actions bot mentioned this pull request Jan 13, 2023
Open
26 tasks
@addaleax addaleax added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 13, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 13, 2023
@nodejs-github-bot nodejs-github-bot merged commit f5dc92c into nodejs:main Jan 13, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in f5dc92c

This was referenced Jan 14, 2023
Open
Open
Open
Open
RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Jan 17, 2023
@codebytere @RafaelGSS
src: remove return after abort
6515511 
PR-URL: nodejs#46172
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@github-actions github-actions bot mentioned this pull request Jan 18, 2023
Open
18 tasks
RafaelGSS pushed a commit that referenced this pull request Jan 20, 2023
@codebytere @RafaelGSS
src: remove return after abort
e85f766 
PR-URL: #46172
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 20, 2023
Merged
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
@codebytere @juanarbol
src: remove return after abort
f20bab0 
PR-URL: #46172
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@juanarbol juanarbol mentioned this pull request Jan 28, 2023
Merged
juanarbol pushed a commit that referenced this pull request Jan 31, 2023
@codebytere @juanarbol
src: remove return after abort
53ecd20 
PR-URL: #46172
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Feb 6, 2023
@codebytere
src: remove return after abort
d06a8c3 
nodejs/node#46172
jkleinsc pushed a commit to electron/electron that referenced this pull request Feb 7, 2023
@electron-roller @codebytere
chore: bump node to v18.14.0 (main) (#37129)
de607cb 
* chore: bump node in DEPS to v18.14.0

* src: add support for externally shared js builtins

nodejs/node#44376

* test: fix test broken under --node-builtin-modules-path

nodejs/node#45894

* build: add option to disable shared readonly heap

nodejs/node#45887

* src: remove unnecessary semicolons

nodejs/node#46171

* src: remove return after abort

nodejs/node#46172

* chore: fixup patch indices

* test_runner: parse yaml

nodejs/node#45815

* src: use simdutf for converting externalized builtins to UTF-16

nodejs/node#46119

* src: rename internal module declaration as internal bindings

nodejs/node#45551

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
@electron-roller @codebytere @khalwa
chore: bump node to v18.14.0 (main) (electron#37129)
a3a8cf5 
* chore: bump node in DEPS to v18.14.0

* src: add support for externally shared js builtins

nodejs/node#44376

* test: fix test broken under --node-builtin-modules-path

nodejs/node#45894

* build: add option to disable shared readonly heap

nodejs/node#45887

* src: remove unnecessary semicolons

nodejs/node#46171

* src: remove return after abort

nodejs/node#46172

* chore: fixup patch indices

* test_runner: parse yaml

nodejs/node#45815

* src: use simdutf for converting externalized builtins to UTF-16

nodejs/node#46119

* src: rename internal module declaration as internal bindings

nodejs/node#45551

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Feb 28, 2023
@electron-roller @codebytere @gecko19
chore: bump node to v18.14.0 (main) (electron#37129)
cb27024 
* chore: bump node in DEPS to v18.14.0

* src: add support for externally shared js builtins

nodejs/node#44376

* test: fix test broken under --node-builtin-modules-path

nodejs/node#45894

* build: add option to disable shared readonly heap

nodejs/node#45887

* src: remove unnecessary semicolons

nodejs/node#46171

* src: remove return after abort

nodejs/node#46172

* chore: fixup patch indices

* test_runner: parse yaml

nodejs/node#45815

* src: use simdutf for converting externalized builtins to UTF-16

nodejs/node#46119

* src: rename internal module declaration as internal bindings

nodejs/node#45551

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Mar 15, 2023
@electron-roller @codebytere @gecko19
chore: bump node to v18.14.0 (main) (electron#37129)
999db88 
* chore: bump node in DEPS to v18.14.0

* src: add support for externally shared js builtins

nodejs/node#44376

* test: fix test broken under --node-builtin-modules-path

nodejs/node#45894

* build: add option to disable shared readonly heap

nodejs/node#45887

* src: remove unnecessary semicolons

nodejs/node#46171

* src: remove return after abort

nodejs/node#46172

* chore: fixup patch indices

* test_runner: parse yaml

nodejs/node#45815

* src: use simdutf for converting externalized builtins to UTF-16

nodejs/node#46119

* src: rename internal module declaration as internal bindings

nodejs/node#45551

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Mar 15, 2023
@electron-roller @codebytere @gecko19
chore: bump node to v18.14.0 (main) (electron#37129)
1323f07 
* chore: bump node in DEPS to v18.14.0

* src: add support for externally shared js builtins

nodejs/node#44376

* test: fix test broken under --node-builtin-modules-path

nodejs/node#45894

* build: add option to disable shared readonly heap

nodejs/node#45887

* src: remove unnecessary semicolons

nodejs/node#46171

* src: remove return after abort

nodejs/node#46172

* chore: fixup patch indices

* test_runner: parse yaml

nodejs/node#45815

* src: use simdutf for converting externalized builtins to UTF-16

nodejs/node#46119

* src: rename internal module declaration as internal bindings

nodejs/node#45551

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaisinTen RaisinTen RaisinTen approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@anonrig anonrig anonrig approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@mhdawson mhdawson Awaiting requested review from mhdawson

Assignees
No one assigned
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++. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@codebytere @nodejs-github-bot @jasnell @addaleax @anonrig @cjihrig @richardlau @RaisinTen