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: add new CHECK macros #20914

Closed
wants to merge 2 commits into from
Closed

src: add new CHECK macros #20914

wants to merge 2 commits into from

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented May 23, 2018 •
edited
Loading

This adds three more CHECK macros to util.h, similar to their definitions within v8. I personally like the added semantics very much, but I can also understand if people think of it as added complexity.

These macros are

  • CHECK_NULL(x) - asserts that x is null
  • CHECK_NOT_NULL(x) - asserts that x is not null
  • CHECK_IMPLIES(a, b) - asserts that b is true if a is true
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@tniessen
src: add CHECK_IMPLIES macro
91346e4 
This change introduces the CHECK_IMPLIES macro
similar to its definition in v8 and replaces instances of
CHECK with CHECK_IMPLIES where it seems appropriate.
@tniessen
src: add CHECK_NULL/CHECK_NOT_NULL macros
4973a6e 
This change introduces CHECK_NULL and CHECK_NOT_NULL macros
similar to their definition in v8 and replaces instances of
CHECK/CHECK_EQ/CHECK_NE with these where it seems appropriate.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels May 23, 2018
@tniessen tniessen changed the title src: add new check macros src: add new CHECK macros May 23, 2018
@tniessen
Copy link
Member Author

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

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 24, 2018
tniessen added a commit that referenced this pull request May 26, 2018
@tniessen
src: add CHECK_IMPLIES macro
13493e9 
This change introduces the CHECK_IMPLIES macro
similar to its definition in v8 and replaces instances of
CHECK with CHECK_IMPLIES where it seems appropriate.

PR-URL: #20914
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
tniessen added a commit that referenced this pull request May 26, 2018
@tniessen
src: add CHECK_NULL/CHECK_NOT_NULL macros
39f2096 
This change introduces CHECK_NULL and CHECK_NOT_NULL macros
similar to their definition in v8 and replaces instances of
CHECK/CHECK_EQ/CHECK_NE with these where it seems appropriate.

PR-URL: #20914
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@tniessen
Copy link
Member Author

Landed in 13493e9, 39f2096.

@tniessen tniessen closed this May 26, 2018
MylesBorins pushed a commit that referenced this pull request May 29, 2018
@tniessen @MylesBorins
src: add CHECK_IMPLIES macro
5e69e1a 
This change introduces the CHECK_IMPLIES macro
similar to its definition in v8 and replaces instances of
CHECK with CHECK_IMPLIES where it seems appropriate.

PR-URL: #20914
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 29, 2018
@tniessen @MylesBorins
src: add CHECK_NULL/CHECK_NOT_NULL macros
902120a 
This change introduces CHECK_NULL and CHECK_NOT_NULL macros
similar to their definition in v8 and replaces instances of
CHECK/CHECK_EQ/CHECK_NE with these where it seems appropriate.

PR-URL: #20914
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request May 29, 2018
Merged
@tniessen tniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2018
@ZYSzys ZYSzys mentioned this pull request Feb 4, 2019
Closed
2 tasks
danbev pushed a commit that referenced this pull request Feb 7, 2019
@ZYSzys @danbev
src: use NULL check macros to check nullptr
93e0c6a 
PR-URL: #25916
Refs: #20914
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
targos pushed a commit that referenced this pull request Feb 10, 2019
@ZYSzys @targos
src: use NULL check macros to check nullptr
5de1034 
PR-URL: #25916
Refs: #20914
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@tniessen @bnoordhuis @jasnell @addaleax @cjihrig @BridgeAR @nodejs-github-bot