Skip to content

src: do not expose simdjson.h in node_config_file.h - #57173

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:gn-simdjson
Feb 24, 2025
Merged

src: do not expose simdjson.h in node_config_file.h#57173
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:gn-simdjson

Conversation

@zcbenz

@zcbenzzcbenz commented Feb 22, 2025

Copy link
Copy Markdown
Contributor

It is not needed, and on Windows files including the header needs some other configs otherwise errors like this may happen:

../../node/deps/simdjson/simdjson.h(12949,7): error: implicitly declaring library function '_BitScanReverse64' with type 'unsigned char (unsigned long *, unsigned long long) noexcept' [-Werror,-Wimplicit-function-declaration]
12949 | if (_BitScanReverse64(&leading_zero, input_num))
| ^
../../node/deps/simdjson/simdjson.h(12949,7): note: include the header <intrin.h> or explicitly provide a declaration for '_BitScanReverse64'
1 error generated.

@nodejs-github-botnodejs-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 Feb 22, 2025
@codecov

codecovBot commented Feb 22, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.36%. Comparing base (90ab559) to head (6843062).
Report is 51 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #57173 +/- ##
=======================================
Coverage 90.36% 90.36% =======================================
Files 629 629 Lines 184373 184373 Branches 36027 36022 -5 =======================================
Hits 166609 166609 - Misses 10907 10915 +8 + Partials 6857 6849 -8

see 25 files with indirect coverage changes

@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 23, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2025
@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Feb 23, 2025

Copy link
Copy Markdown
Collaborator

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

@richardlaurichardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2025
@nodejs-github-bot
nodejs-github-bot merged commit 8a9c3c2 into nodejs:mainFeb 24, 2025
@nodejs-github-bot

nodejs-github-bot commented Feb 24, 2025

Copy link
Copy Markdown
Collaborator

Landed in 8a9c3c2

@zcbenz
zcbenz deleted the gn-simdjson branch February 24, 2025 23:24
@targostargos added dont-land-on-v18.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Feb 25, 2025
@zcbenzzcbenz mentioned this pull request Feb 25, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
@zcbenz@RafaelGSS
src: do not expose simdjson.h in node_config_file.h
28ad95f
PR-URL: #57173
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@zcbenz@RafaelGSS
src: do not expose simdjson.h in node_config_file.h
fc90f88
PR-URL: #57173
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@zcbenz@RafaelGSS
src: do not expose simdjson.h in node_config_file.h
076a99f
PR-URL: #57173
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@lpincalpincalpinca approved these changes
@anonriganonriganonrig approved these changes
@richardlaurichardlaurichardlau approved these changes
@StefanStojanovicStefanStojanovicStefanStojanovic approved these changes

Assignees

No one assigned

Labels

author readyPRs 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++.dont-land-on-v22.xPRs that should not land on the v22.x-staging branch and should not be released in v22.x.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@zcbenz@nodejs-github-bot@jasnell@lpinca@anonrig@richardlau@StefanStojanovic@targos@aduh95@marco-ippolito