Skip to content

fix(clang-tidy): replace C-style arrays with std::array - #1233

Merged
limdor merged 1 commit into
eclipse-score:mainfrom
castler:fix_modernize_avoid_c_arrays
Oct 2, 2026
Merged

limdor merged 1 commit into
eclipse-score:mainfrom
castler:fix_modernize_avoid_c_arrays

Conversation

@castler

@castler castler commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes all 11 modernize-avoid-c-arrays findings.

  • std::array for padding (generic skeleton apps), client_pipe_fds (flock_test.cpp), last_shm_open_path_ (sample_sender_receiver.cpp) and the placement-new storage in the EXPORT_MW_COM_TYPE macro (7 findings in the generated *_gen.cpp files).
  • runtime.cpp: the deprecated InitializeRuntime(argc, argv[]) keeps its signature; its NOLINTNEXTLINE was separated from the declaration by a comment line and therefore did not apply. It now sits directly above the function.

Verification

  • clang-tidy: no remaining findings for this check
  • bazel build --config=ci //score/... and bazel test //score/mw/com/... pass

Comment thread score/mw/com/runtime.cpp
// NOLINTNEXTLINE(modernize-avoid-c-arrays):C-style array tolerated for command line arguments. This API is deprecated
// and it will be removed.
// C-style array tolerated for command line arguments. This API is deprecated and it will be removed.
// NOLINTNEXTLINE(modernize-avoid-c-arrays)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The problem doing it like that is that it would then fail the check that a suppression should have an argumentation. Not sure what would be the right solution.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not believe that we have such check right now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, lets cross that bridge when we come to it. For the moment I will approve it and once we have a checker and this violates, we should be able to fine the argumentation quite easy because it is just above.
The only quesiton remaining would be, do we think that having a checker that all suppressions should have a justification attached is important? If yes I would create a ticket for it to not forget and we can merge this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As the safety argumentation will be done via CodeQL - I don't think it is worth the hassle

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair enough, it makes sense

limdor
limdor previously approved these changes Oct 2, 2026
@castler
castler added this pull request to the merge queue Oct 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 2, 2026
@castler
castler force-pushed the fix_modernize_avoid_c_arrays branch from a72347f to 4a5a890 Compare October 2, 2026 12:15
@limdor
limdor enabled auto-merge October 2, 2026 12:17
@limdor
limdor added this pull request to the merge queue Oct 2, 2026
Merged via the queue into eclipse-score:main with commit 0bec597 Oct 2, 2026
19 checks passed
@limdor
limdor deleted the fix_modernize_avoid_c_arrays branch October 2, 2026 12:45
Resolve all 11 modernize-avoid-c-arrays findings:
- std::array for padding members, the pipe fds, the shm path buffer and the
  placement-new storage in the EXPORT_MW_COM_TYPE macro (covers the 7
  findings in the generated *_gen.cpp files).
- runtime.cpp: the deprecated InitializeRuntime(argc, argv[]) must keep its
  signature; move the NOLINTNEXTLINE directly above the declaration so it
  actually applies.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
etas-contrib-automation Bot pushed a commit to etas-contrib/score_communication that referenced this pull request Oct 5, 2026
…ipse-score#1250)

## Summary
Fixes the remaining
`cppcoreguidelines-pro-bounds-array-to-pointer-decay` findings in
`sample_sender_receiver.cpp`:
- `ToStringImpl` takes its variadic arguments by `const Args&...`, so
string literals no longer decay when recursing.
- The one remaining decay (`o << t` for string literals) gets a
justified `NOLINTNEXTLINE`.

The other 9 alerts of this check (`flock_test.cpp`,
`last_shm_open_path_`, and the `*_gen.cpp` macro storage) are C-style
arrays that are replaced by `std::array` in eclipse-score#1233; they disappear once
that is merged (verified by linting that branch).

## Verification
- clang-tidy on this branch: only the findings covered by eclipse-score#1233 remain
- `bazel test //score/mw/com/test/common_test_resources/...
//score/mw/com/test/bigdata/...` pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants