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.

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: define noreturn attribute for windows #31467

Closed
wants to merge 1 commit into from

Conversation

lxbndr
Copy link
Contributor

@lxbndr lxbndr commented Jan 22, 2020

As MSVC has it's own noreturn attribute, wouldn't it be useful to utilize it in NAPI_NO_RETURN macro?

I believe it will have almost zero impact for majority of addons, but code analysis tools would be thankful.

I am working on addon written in Swift. It appears that C module importer actually handles noreturn attributes to generate corresponding Swift interface. Which in turn affects type inference. As a result I got different compiler behavior on Windows for exactly same code as on Darwin platform.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows [commit guidelines]

NOTE: I had 17 failing tests on master before this change. I have 17 failing tests after this change. Took this as "pass". Would be glad if anyone correct me if tests really require attention here.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels Jan 22, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Trott pushed a commit that referenced this pull request Jan 25, 2020
PR-URL: #31467
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Trott commented Jan 25, 2020

Landed in 3188afe

Thanks for the contribution! 🎉

@Trott Trott closed this Jan 25, 2020
@lxbndr lxbndr deleted the msvc-napi-noreturn branch January 25, 2020 20:18
codebytere pushed a commit that referenced this pull request Feb 17, 2020
PR-URL: #31467
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 15, 2020
PR-URL: #31467
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
PR-URL: #31467
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants