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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for building with feature gates ["simd"] #534

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jul 9, 2021

Which issue does this PR close?

re #458

Built on #532 and #533 so review them first

Rationale for this change

There are reports #458 that user applications can't be built against arrow using the simd feature gate and I wanted to increase test coverage. However, arrow still doesn't seem to build from crates.io 馃

What changes are included in this PR?

tests and clean up some warnings

Are there any user-facing changes?

no

@codecov-commenter
Copy link

Codecov Report

Merging #534 (7ef5319) into master (f1fb2b1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #534   +/-   ##
=======================================
  Coverage   82.60%   82.60%           
=======================================
  Files         167      167           
  Lines       45984    45984           
=======================================
  Hits        37984    37984           
  Misses       8000     8000           
Impacted Files Coverage 螖
arrow/src/compute/kernels/comparison.rs 95.84% <酶> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update f1fb2b1...7ef5319. Read the comment docs.

@alamb alamb force-pushed the alamb/fix-simd-flag-builds branch from 7ef5319 to 0eb3c3b Compare July 12, 2021 19:28
@alamb alamb marked this pull request as ready for review July 12, 2021 19:31
@@ -591,7 +591,7 @@ where

let bitmask = T::mask_to_u64(&simd_result);
let bytes = bitmask.to_le_bytes();
&result_slice[0..lanes / 8].copy_from_slice(&bytes[0..lanes / 8]);
result_slice[0..lanes / 8].copy_from_slice(&bytes[0..lanes / 8]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unrelated, was it a needless borrow clippy lint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- sorry -- I should have mentioned that. When I was testing the project with simd locally this was flagged as a compiler warning so I fixed it.

@alamb alamb merged commit d6334aa into apache:master Jul 13, 2021
@alamb alamb deleted the alamb/fix-simd-flag-builds branch July 13, 2021 10:54
@alamb alamb added the development-process Related to development process of arrow-rs label Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate development-process Related to development process of arrow-rs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants