Skip to content

hmon: global fixes to HMON#108

Merged
pawelrutkaq merged 1 commit intoeclipse-score:mainfrom
qorix-group:arkjedrz_global-fixes
Mar 6, 2026
Merged

hmon: global fixes to HMON#108
pawelrutkaq merged 1 commit intoeclipse-score:mainfrom
qorix-group:arkjedrz_global-fixes

Conversation

@arkjedrz
Copy link
Contributor

@arkjedrz arkjedrz commented Mar 5, 2026

  • Rename __drop_by_rust_impl to _drop_by_rust_impl.
    • Double underscore is reserved in C++.
  • Rename #[no_mangle] to #[unsafe(no_mangle)].
  • Add const constructors for tags.

@github-actions
Copy link

github-actions bot commented Mar 5, 2026

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: 70fd826e-7fb2-4711-b582-f630306a3c69
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'score_rust_policies', the root module requires module version score_rust_policies@0.0.3, but got score_rust_policies@0.0.5 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (52 packages loaded, 9 targets configured)

Analyzing: target //:license-check (93 packages loaded, 16 targets configured)

Analyzing: target //:license-check (145 packages loaded, 2642 targets configured)

Analyzing: target //:license-check (153 packages loaded, 5754 targets configured)

Analyzing: target //:license-check (163 packages loaded, 7875 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (165 packages loaded, 7901 targets configured)

Analyzing: target //:license-check (168 packages loaded, 9789 targets configured)

Analyzing: target //:license-check (169 packages loaded, 9913 targets configured)

INFO: Analyzed target //:license-check (170 packages loaded, 10039 targets configured).
[10 / 16] Creating runfiles tree bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/score_tooling+/dash/tool/formatters/dash_format_converter.runfiles [for tool]; 0s local ... (2 actions, 1 running)
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] [Prepa] Building license.check.license_check.jar ()
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: 26.284s, Critical Path: 2.70s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 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>]

Copy link

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

This PR fixes two correctness issues in the health monitoring library's FFI layer:

  1. #[no_mangle]#[unsafe(no_mangle)]: Since Rust 1.82, no_mangle must be wrapped in unsafe(...) to acknowledge that exporting an unmangled symbol is an unsafe operation. All 15 occurrences across both FFI files are updated.
  2. __drop_by_rust_impl_drop_by_rust_impl: Names beginning with two underscores are reserved at all scopes in C++, causing undefined behavior. The rename uses a single leading underscore inside class scope, which is valid.

Changes:

  • All #[no_mangle] attributes in the Rust FFI layer updated to #[unsafe(no_mangle)].
  • Double-underscore C++ method name __drop_by_rust_impl renamed to _drop_by_rust_impl in both the declaration and the call site.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/health_monitoring_lib/rust/ffi.rs Updated 7 #[no_mangle]#[unsafe(no_mangle)] on all exported C FFI functions
src/health_monitoring_lib/rust/deadline/ffi.rs Updated 8 #[no_mangle]#[unsafe(no_mangle)] on all exported C FFI functions
src/health_monitoring_lib/cpp/include/score/hm/deadline/deadline_monitor.h Renamed method __drop_by_rust_impl_drop_by_rust_impl in DeadlineMonitorBuilder
src/health_monitoring_lib/cpp/include/score/hm/common.h Updated the CRTP call site __drop_by_rust_impl()_drop_by_rust_impl() in RustDroppable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@github-actions
Copy link

github-actions bot commented Mar 5, 2026

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

- Rename `__drop_by_rust_impl` to `_drop_by_rust_impl`.
  - Double underscore is reserved in C++.
- Rename `#[no_mangle]` to `#[unsafe(no_mangle)]`.
- Add const constructors for tags.
@arkjedrz arkjedrz force-pushed the arkjedrz_global-fixes branch from b2f276d to 6c3ed43 Compare March 5, 2026 13:42
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 5, 2026 13:42 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 5, 2026 13:42 — with GitHub Actions Inactive
@pawelrutkaq pawelrutkaq merged commit 419588e into eclipse-score:main Mar 6, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants