Skip to content

an explicit platform split matching the helper header's own condition. - #587

Open
fujitatomoya wants to merge 1 commit into
rollingfrom
replaces/pull/586
Open

an explicit platform split matching the helper header's own condition.#587
fujitatomoya wants to merge 1 commit into
rollingfrom
replaces/pull/586

Conversation

@fujitatomoya

Copy link
Copy Markdown
Collaborator

Description

replaces #586

note: I first applied ATOMIC_VAR_INIT(-1) as suggested, but when I tested it in C23 mode (gcc -std=gnu2x), it failed — ATOMIC_VAR_INIT is already removed from this toolchain's <stdatomic.h>, producing "implicit declaration of function 'ATOMIC_VAR_INIT'" followed by "initializer element is not constant". which isn't theoretical. I switched to the fallback to have an explicit platform split matching the helper header's own condition.

Fixes # (issue)

Is this user-facing behavior change?

No

Did you use Generative AI?

No

Additional Information

Signed-off-by: Tomoya Fujita <fujita.tomoya@triorb.co.jp>
@mergify

mergify Bot commented Aug 19, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fujitatomoya
fujitatomoya requested a review from ahcorde August 19, 2026 04:22
@fujitatomoya

Copy link
Copy Markdown
Collaborator Author

Pulls: #587
Gist: https://gist.githubusercontent.com/fujitatomoya/c7dc0267a7c77d0fcc0a827d492d036a/raw/cc34078e8e7c2d098313e1b7058f0062ee6c68ab/ros2.repos
BUILD args: --packages-above-and-dependencies rcutils
TEST args: --packages-above rcutils
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/20102

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

i think this is a best option we got, any thoughs or better idea?

@yukaitung

yukaitung commented Aug 19, 2026

Copy link
Copy Markdown

Without PR
Screenshot 2026-08-19 at 12 58 02
With PR
Screenshot 2026-08-19 at 13 00 22
The package builds successfully, lgtm, please backport to lyrical

This fix should be the best option since there is no way to ask Apple Clang to bypass it.

@fujitatomoya

Copy link
Copy Markdown
Collaborator Author
  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

#if defined(_WIN32) && !defined(__MINGW64__)
static atomic_int_least64_t g_rcutils_fault_injection_count = {-1};
#else
static atomic_int_least64_t g_rcutils_fault_injection_count = -1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see a very similar one in rcl when compiling locally https://github.com/ros2/rcl/blob/8f6b3720079ed8371e1e9688bae9bb247ffde5af/rcl/src/rcl/init.c#L43, I think we want to update that as well.

rcl/rcl/src/rcl/init.c:43:53: warning: macro 'ATOMIC_VAR_INIT' has been marked as deprecated [-Wdeprecated-pragma]
   43 | static atomic_uint_least64_t __rcl_next_unique_id = ATOMIC_VAR_INIT(1);
      |                                                     ^
.pixi/envs/default/lib/clang/19/include/stdatomic.h:62:41: note: macro marked 'deprecated' here
   62 | #pragma clang deprecated(ATOMIC_VAR_INIT)

@wentasah

Copy link
Copy Markdown

This works for me. Thanks!

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.

4 participants