MOD-10017 Rust Counter result processor#6261
Merged
JonasKruckenberg merged 46 commits intomasterfrom Jun 23, 2025
Merged
Conversation
271ff35 to
4a509f2
Compare
285da70 to
93c49b4
Compare
93c49b4 to
1a33a65
Compare
bc559bd to
0d95e10
Compare
0d95e10 to
887918d
Compare
887918d to
84ea3a6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6261 +/- ##
==========================================
+ Coverage 88.83% 88.90% +0.06%
==========================================
Files 246 250 +4
Lines 41051 41364 +313
Branches 3483 3556 +73
==========================================
+ Hits 36469 36776 +307
Misses 4539 4539
- Partials 43 49 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
85be6cc to
690a662
Compare
690a662 to
c29d7d0
Compare
01f65da to
5605cc3
Compare
8c98d07 to
3c9c9ab
Compare
6b0f420 to
4a51e18
Compare
Itzikvaknin
requested changes
Jun 19, 2025
Collaborator
Itzikvaknin
left a comment
There was a problem hiding this comment.
Please related to the monitoring comments
We need to make sure we integrate "rustic" SearchResult once its ported
| /// | ||
| /// - The caller must never move the allocated result processor from its original allocation. | ||
| /// - The caller must ensure to call the `Free` VTable function to properly destroy the type. | ||
| #[unsafe(no_mangle)] |
Collaborator
There was a problem hiding this comment.
isnt it always unsafe?
Comment on lines
+45
to
+49
| impl Default for Counter { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| } | ||
| } |
| self.count += 1; | ||
|
|
||
| // Safety: This function should only be called on initialized SearchResults. Luckily, | ||
| // a ResultProcessor returning `RPStatus_RS_RESULT_OK` means "Result is filled with valid data" |
Collaborator
There was a problem hiding this comment.
How is it inferred? (is_some() -> RPStatus_RS_RESULT_OK)
Itzikvaknin
approved these changes
Jun 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the changes in the pull request
This PR ports the RPCounter to Rust without exposing it to C yet. This implements all the machinery and setup, as well as tests required to make sure the new RP behaves exactly as the old RP. Enabling the new RP and switching the codebase over is left for a followup PR.
stacked on top of #6250
Note:
A lot of the design is transient, once more code has been ported to Rust we will revisit certain APIs and update them accordingly. (See MOD-9920, MOD-10230, and MOD-10229)