ABI-compatible Option and Result types by awillenbuecher-xq-tec · Pull Request #41 · eclipse-score/baselibs_rust

awillenbuecher-xq-tec · 2025-12-17T13:13:34Z

Notes for Reviewer

The types are called InlineOption and InlineResult to be consistent with the other ABI-compatible data structures in the library. Like them, these types are intended to be imported in the mw_com library, where the zero-copy traits are impld for the wrappers.

Pre-Review Checklist for the PR Author

PR title is short, expressive and meaningful
Commits are properly organized
Relevant issues are linked in the References section
Tests are conducted
Unit tests are added

Checklist for the PR Reviewer

Commits are properly organized and messages are according to the guideline
Unit tests have been written for new behavior
Public API is documented
PR title describes the changes

Post-review Checklist for the PR Author

All open points are addressed and tracked via issues

References

Closes #40

github-actions · 2025-12-17T13:14:26Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 89e5ecd5-f61d-4c2a-a80a-75a969b90627
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rust_qnx8_toolchain+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-eQOopREOYCL5vtTb6c1cwZrql4GVrJ1FqgxarQRe1xs="
DEBUG: Repository rust_qnx8_toolchain+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:431:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'bazel_skylib', the root module requires module version bazel_skylib@1.7.1, but got bazel_skylib@1.8.1 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_cc', the root module requires module version rules_cc@0.1.1, but got rules_cc@0.1.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'aspect_rules_lint', the root module requires module version aspect_rules_lint@1.0.3, but got aspect_rules_lint@1.5.3 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'buildifier_prebuilt', the root module requires module version buildifier_prebuilt@7.3.1, but got buildifier_prebuilt@8.2.0.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'score_docs_as_code', the root module requires module version score_docs_as_code@2.0.2, but got score_docs_as_code@2.2.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (34 packages loaded, 10 targets configured)

Analyzing: target //:license-check (131 packages loaded, 617 targets configured)

Analyzing: target //:license-check (147 packages loaded, 1835 targets configured)

Analyzing: target //:license-check (147 packages loaded, 1835 targets configured)

Analyzing: target //:license-check (152 packages loaded, 4833 targets configured)

Analyzing: target //:license-check (155 packages loaded, 8887 targets configured)

Analyzing: target //:license-check (155 packages loaded, 8887 targets configured)

INFO: Analyzed target //:license-check (156 packages loaded, 9013 targets configured).
INFO: From Generating Dash formatted dependency file ...:
WARNING: No packages found in Cargo.lock.
INFO: Successfully converted 0 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 25.575s, Critical Path: 0.42s
INFO: 14 processes: 5 disk cache hit, 9 internal.
INFO: Build completed successfully, 14 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2025-12-17T13:15:53Z

The created documentation from the pull request is available at: docu-html

pawelrutkaq · 2025-12-17T13:45:50Z

src/containers/inline/option.rs

+    /// Returns an optional reference to the contained value.
+    pub const fn as_ref(&self) -> Option<&T> {
+        if self.is_some {
+            Some(unsafe { &*self.value.as_ptr() })


safet why this is safe to derefence maybeuninit ptr (just that we checked above is_some)

other unsafe places too

You're right. There's a (clippy?) lint that enforces "SAFETY" comments, maybe we should enable that.

pawelrutkaq · 2025-12-17T13:53:23Z

src/containers/inline/option.rs

+#[repr(C)]
+pub struct InlineOption<T: Copy> {
+    value: MaybeUninit<T>,
+    is_some: bool,


Shall we specify this as reprC enum enforced to u8 with states instead realing on bool? Or use u8 and do checking on value before using it ?

This would be possible, and the ABI tool actually translates enums from the description language into repr(u8) enums in Rust. Unfortunately, the ABI types feature request dictates this memory layout; we could change the feature request, but I don't think we'd gain any benefits from this.

pawelrutkaq · 2025-12-17T13:59:22Z

src/containers/inline/result.rs

+    /// Returns `true` if and only if this instance represents failure.
+    pub fn is_err(&self) -> bool {
+        !self.is_ok
+    }


wonder if we shall expose this or rather fallback this all into buildin types via as_ref and say this container have no direct Result API ?

I guess there's no big advantage to having these methods, except that they generate less intermediate code which has to be optimized away by the compiler. I can remove them if you want.

lets remove then for consistent usage ? and why we dont use deref?

Done, as discussed

awillenbuecher-xq-tec requested a review from pawelrutkaq December 17, 2025 13:14

pawelrutkaq requested changes Dec 17, 2025

View reviewed changes

awillenbuecher-xq-tec force-pushed the adwi_option-result branch from ab6489b to 772b4e2 Compare December 17, 2025 16:59

awillenbuecher-xq-tec requested a review from pawelrutkaq December 17, 2025 17:02

awillenbuecher-xq-tec force-pushed the adwi_option-result branch from 772b4e2 to 3275d90 Compare December 18, 2025 08:12

ABI-compatible Option and Result

46494a6

awillenbuecher-xq-tec force-pushed the adwi_option-result branch from 3275d90 to 46494a6 Compare December 18, 2025 08:57

awillenbuecher-xq-tec had a problem deploying to workflow-approval December 18, 2025 08:58 — with GitHub Actions Failure

awillenbuecher-xq-tec temporarily deployed to workflow-approval December 18, 2025 09:12 — with GitHub Actions Inactive

Include QNX toolchain credentials

acba861

awillenbuecher-xq-tec force-pushed the adwi_option-result branch from 8095d3e to acba861 Compare December 18, 2025 09:18

awillenbuecher-xq-tec temporarily deployed to workflow-approval December 18, 2025 09:18 — with GitHub Actions Inactive

pawelrutkaq approved these changes Dec 18, 2025

View reviewed changes

pawelrutkaq merged commit 9ee64ac into eclipse-score:main Dec 18, 2025
12 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

ABI-compatible Option and Result types#41

ABI-compatible Option and Result types#41
pawelrutkaq merged 2 commits intoeclipse-score:mainfrom
qorix-group:adwi_option-result

awillenbuecher-xq-tec commented Dec 17, 2025

Uh oh!

github-actions bot commented Dec 17, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 17, 2025

Uh oh!

pawelrutkaq Dec 17, 2025

Uh oh!

pawelrutkaq Dec 17, 2025

Uh oh!

awillenbuecher-xq-tec Dec 17, 2025

Uh oh!

pawelrutkaq Dec 17, 2025

Uh oh!

awillenbuecher-xq-tec Dec 17, 2025

Uh oh!

pawelrutkaq Dec 17, 2025

Uh oh!

awillenbuecher-xq-tec Dec 17, 2025

Uh oh!

pawelrutkaq Dec 18, 2025

Uh oh!

awillenbuecher-xq-tec Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

awillenbuecher-xq-tec commented Dec 17, 2025

Notes for Reviewer

Pre-Review Checklist for the PR Author

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

References

Uh oh!

github-actions bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Dec 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Dec 17, 2025 •

edited

Loading