Skip to content

FFI Layer: Implement RSValueTrait for FFI and implement the C entry point - MOD-9985#6459

Merged
DarthB merged 1 commit intomasterfrom
rp-rsvalue-crate
Jul 21, 2025
Merged

FFI Layer: Implement RSValueTrait for FFI and implement the C entry point - MOD-9985#6459
DarthB merged 1 commit intomasterfrom
rp-rsvalue-crate

Conversation

@DarthB
Copy link
Copy Markdown
Contributor

@DarthB DarthB commented Jul 7, 2025

What is part of this PR?

This PR adds the FFI layer that is used by the upcoming RSSortingVector port. It allows C to use the RSSortingVector Rust implementation.

For this, the type RSValueFFI is created. It implements the trait RSValueTrait and is used temporarily until the RSValue type is ported to Rust.

Referenced PRs

Stacked on top of: #6311

Upcoming PR:
Full swap of C with Rust Implementation: #6397

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 7, 2025

Codecov Report

Attention: Patch coverage is 0% with 179 lines in your changes missing coverage. Please review.

Project coverage is 89.28%. Comparing base (4ccff30) to head (c4b2c35).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
...arch_rs/c_entrypoint/sorting_vector_ffi/src/lib.rs 0.00% 113 Missing ⚠️
src/redisearch_rs/value/src/lib.rs 0.00% 66 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6459      +/-   ##
==========================================
- Coverage   89.67%   89.28%   -0.39%     
==========================================
  Files         257      259       +2     
  Lines       42217    42798     +581     
  Branches     4519     5089     +570     
==========================================
+ Hits        37857    38212     +355     
- Misses       4291     4503     +212     
- Partials       69       83      +14     
Flag Coverage Δ
flow 81.97% <ø> (-0.15%) ⬇️
unit 48.31% <0.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DarthB DarthB force-pushed the rp-rsvalue-crate branch from 89562b1 to 1ffff0d Compare July 8, 2025 07:05
@DarthB DarthB changed the title Extracting of RSValue Trait and it's implementation for FFI (calling Rust from C) - MOD-9985 FFI Layer: Implement RSValueTrait for FFI and implement the C entry point - MOD-9985 Jul 8, 2025
@DarthB DarthB force-pushed the rp-rsvalue-crate branch from 1ffff0d to 51be97c Compare July 8, 2025 07:24
@DarthB DarthB force-pushed the rp-rsvalue-crate branch from 1bc9a87 to 8239a54 Compare July 8, 2025 08:28
@DarthB DarthB changed the base branch from master to rp-rssortingvector July 8, 2025 15:03
@DarthB DarthB requested a review from Copilot July 8, 2025 15:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements a new FFI layer exposing the Rust RSValue and RSSortingVector functionality to C.

  • Adds RSValueFFI wrapper over C’s RSValue with reference counting and RSValueTrait implementation.
  • Generates and updates the C header (sorting_vector_rs.h) via cbindgen.
  • Provides C entry points (RSSortingVector_* functions) in Rust for creating, querying, and freeing sorting vectors.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/redisearch_rs/value/src/lib.rs FFI wrapper RSValueFFI implementing RSValueTrait
src/redisearch_rs/headers/sorting_vector_rs.h Auto-generated C header for RSSortingVector API
src/redisearch_rs/c_entrypoint/sorting_vector_ffi/src/lib.rs Rust implementations of C entry points for sorting vector operations
src/redisearch_rs/c_entrypoint/sorting_vector_ffi/cbindgen.toml Configuration for header generation via cbindgen
src/redisearch_rs/c_entrypoint/sorting_vector_ffi/build.rs Build script invoking cbindgen
Comments suppressed due to low confidence (1)

src/redisearch_rs/c_entrypoint/sorting_vector_ffi/src/lib.rs:165

  • The attribute #[unsafe(no_mangle)] is invalid Rust syntax. It should be #[no_mangle] above the function signature.
#[unsafe(no_mangle)]

@github-actions github-actions bot added the size:L label Jul 9, 2025
@DarthB DarthB force-pushed the rp-rsvalue-crate branch 2 times, most recently from 3c2ed44 to ac79dd0 Compare July 9, 2025 10:09
@DarthB DarthB requested a review from JonasKruckenberg July 10, 2025 10:46
Base automatically changed from rp-rssortingvector to master July 10, 2025 14:47
@DarthB DarthB force-pushed the rp-rsvalue-crate branch 3 times, most recently from a3f0bc5 to 41460a9 Compare July 14, 2025 13:57
@DarthB DarthB force-pushed the rp-rsvalue-crate branch from 41460a9 to 2bc4afb Compare July 14, 2025 14:59
Copy link
Copy Markdown
Collaborator

@nafraf nafraf left a comment

Choose a reason for hiding this comment

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

Just a question in the autogenerated file: src/redisearch_rs/headers/sorting_vector_rs.h

@DarthB DarthB force-pushed the rp-rsvalue-crate branch from 2bc4afb to c4b2c35 Compare July 21, 2025 10:08
@DarthB DarthB added this pull request to the merge queue Jul 21, 2025
Merged via the queue into master with commit 45c02e4 Jul 21, 2025
16 of 17 checks passed
@DarthB DarthB deleted the rp-rsvalue-crate branch July 21, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants