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: do not expose simdjson.h in node_config_file.h #57173

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Feb 22, 2025

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-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 Feb 22, 2025
Copy link

codecov bot commented Feb 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.36%. Comparing base (90ab559) to head (6843062).
Report is 32 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

@aduh95 aduh95 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-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2025
@nodejs-github-bot nodejs-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:main Feb 24, 2025
70 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 8a9c3c2

@zcbenz zcbenz deleted the gn-simdjson branch February 24, 2025 23:24
@targos targos added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.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. dont-land-on-v23.x PRs that should not land on the v23.x-staging branch and should not be released in v23.x. labels Feb 25, 2025
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++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.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. dont-land-on-v23.x PRs that should not land on the v23.x-staging branch and should not be released in v23.x. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants