Repository navigation
fix(clang-tidy): resolve misc-use-anonymous-namespace findings - #1000
Merged
castler merged 1 commit intoAug 24, 2026
Merged
Conversation
castler
requested review from
LittleHuba,
bemerybmw,
crimson11,
hoe-jo and
limdor
as code owners
August 24, 2026 13:10
castler
marked this pull request as draft
August 24, 2026 13:15
castler
force-pushed
the
fix/clang-tidy-anonymous-namespace
branch
2 times, most recently
from
August 24, 2026 13:28
4a6b2bd to
1a91077
Compare
castler
marked this pull request as ready for review
August 24, 2026 13:44
castler
enabled auto-merge
August 24, 2026 13:44
clang-tidy's misc-use-anonymous-namespace check does not provide an automatic FixIt (confirmed both via clang-tidy's own docs and by manually reproducing the exact --fix invocation outside Bazel: clang-tidy exits 0, reports the findings as plain warnings, but never rewrites the file). The aspect_rules_lint autofix pipeline was therefore correctly producing empty patches - there was no pipeline bug to fix, so these 74 findings across 24 files needed to be applied manually. Tutorial/doc files (22 files, chapters 1-10 and 12, consumer/provider pairs): move file-scope `static` variables and functions into an unnamed namespace and drop the (now redundant) `static` keyword, per the check's own recommendation. Rust FFI bridge registry macros (`BEGIN_EXPORT_MW_COM_INTERFACE`, `EXPORT_MW_COM_EVENT` in registry_bridge_macro.h): the `static ..._reg_instance;` declarations generated by these macros are the source of the remaining findings in vehicle_gen.cpp and bigdata_com_api_gen.cpp - the only two call sites of these macros in the repository. Wrapped the generated declarations in a nested anonymous namespace (inside the macro's own named `id##_detail` namespace) and dropped `static`, fixing the finding at its single root cause instead of touching the call sites. Verified via `bazel build --config=clang-tidy` over the affected scope (0 remaining misc-use-anonymous-namespace findings) and a plain `bazel build` of the same targets (compiles cleanly, including the Rust FFI bridge consumers). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
crimson11
approved these changes
Aug 24, 2026
github-project-automation
Bot
moved this from Backlog
to Done
in COM - Communication FT
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_communication
that referenced
this pull request
Aug 24, 2026
…ss (modernize-use-equals-delete) (eclipse-score#1001) ## Summary Fixes all 204 `modernize-use-equals-delete` clang-tidy findings, all of which report "deleted member function should be public". ## Root cause This category of finding has **no available FixIt** in clang-tidy: the check's autofix only adds `= delete` to a non-deleted function — it never relocates a declaration across access specifiers. Every flagged function here was already `= delete`, just placed under `protected`/`private` instead of `public`, so a manual fix was required (same category of limitation previously found for `misc-use-anonymous-namespace` in eclipse-score#1000). ## Scope 204 raw findings across 41 `.cpp` translation units collapse to **8 header files** where the flagged declarations actually live: - `score/message_passing/i_client_connection.h` - `score/message_passing/i_client_factory.h` - `score/message_passing/i_server.h` - `score/message_passing/i_server_connection.h` - `score/message_passing/i_server_factory.h` - `score/message_passing/i_shared_resource_engine.h` - `score/mw/com/impl/proxy_base.h` - `score/mw/com/impl/skeleton_event_base.h` ## Fix Moved the deleted copy/move constructor and assignment operator declarations (with their doc comments) into each class's `public:` section, right after the destructor. Non-deleted (defaulted) constructors/destructors stay in their original `protected:` section. For `proxy_base.h` and `skeleton_event_base.h`, only the deleted **copy** members were relocated — the defaulted **move** members correctly remain protected (unflagged, unchanged). ## Verification - `bazel build --config=clang-tidy` across `score/mw/com/...`, `score/message_passing/...`, `score/memory/...`, `quality/integration_testing/...` → **0 remaining `modernize-use-equals-delete` findings**, no new findings introduced. - Plain `bazel build` over the same scope → compiles cleanly, no regressions. This is a standalone PR per the maintainers' preference to keep each distinct clang-tidy concern isolated (see eclipse-score#995, eclipse-score#999, eclipse-score#1000 for prior examples in this series).
marcmo
pushed a commit
to esrlabs/score_communication
that referenced
this pull request
Aug 26, 2026
…ndings (eclipse-score#1012) ## Summary Fixes all `bugprone-suspicious-stringview-data-usage` clang-tidy findings in the codebase. ## Root cause All findings occur at call sites of `score::json::Object::find()`. `score::json::Object` is a `std::map<StringComparisonAdaptor, Any>`, and `StringComparisonAdaptor` has an implicit constructor from `std::string_view`. Despite this, many call sites narrowed a `std::string_view` key down to a raw `const char*` via `.data()` before calling `find()`: ```cpp constexpr auto kHvSocketConfigurationKey = "hypervisor-socket"sv; ... const auto socket_entry = obj.find(kHvSocketConfigurationKey.data()); ``` `bugprone-suspicious-stringview-data-usage` flags this pattern because `.data()` discards the view's length and the result is not guaranteed to be null-terminated in general — passing the raw `char*` on to an API that expects a null-terminated C-string is a latent bug, even though in these specific cases the views happen to originate from null-terminated string literals. ## Fix Pass the `std::string_view` key directly to `find()` instead of calling `.data()` on it: ```cpp const auto socket_entry = obj.find(kHvSocketConfigurationKey); ``` This relies on the existing implicit `StringComparisonAdaptor(const std::string_view&)` conversion, removes the unsafe pattern, and is a pure call-site simplification with no behavioral change (same key comparison semantics, just without the intermediate C-string narrowing). ## Files changed - `score/mw/com/gateway/transport_layer/sample/configuration/sample_transport_config_parser.cpp` - `score/mw/com/gateway/gateway_application/configuration/gateway_config_parser.cpp` - `score/mw/com/impl/configuration/configuration_json_parsing_strategy.cpp` 53 call sites fixed across the three files (5 + 5 + 43 respectively). ## Verification - Full `bazel build //score/mw/com/... //score/memory/... //score/message_passing/...` passes. - Scoped clang-tidy build isolated to `bugprone-suspicious-stringview-data-usage` shows **0 findings** (down from 53). - All 51 tests under `score/mw/com/impl/configuration/...` and `score/mw/com/gateway/...` pass. ## Scope This PR intentionally contains **only** `bugprone-suspicious-stringview-data-usage` fixes, per the project convention of one clang-tidy check per PR (see eclipse-score#995, eclipse-score#999, eclipse-score#1000, eclipse-score#1001, eclipse-score#1003).
etas-contrib-automation Bot
pushed a commit
to etas-contrib/score_communication
that referenced
this pull request
Aug 26, 2026
…clipse-score#1014) ## Summary Fixes all `cppcoreguidelines-init-variables` clang-tidy findings (11 findings across 4 files). ## Approach Initializes each flagged variable at its point of declaration instead of leaving it default-constructed/uninitialized, per the C++ Core Guidelines' recommendation that every variable be given a well-defined value as soon as it is declared. All changes are pure initialization additions — no behavioral change, since every affected variable is fully overwritten before its first read in the original code, and the chosen initial values (enum default, `0`, `false`) match the variables' subsequent overwritten semantics. ## Fixes - **`unix_domain_engine.cpp`**: `PipeEvent pipe_event` is an out-parameter for a raw `read()` syscall that always overwrites it before use; initialized to `PipeEvent::QUIT` (a valid enumerator) as a safe default. - **`unix_domain_server.cpp`**: `std::uint8_t code` is an out-parameter for `ReceiveProtocolMessage()`; zero-initialized. - **`flock_test.cpp`**: `FILE* stream` was declared then immediately assigned from `fdopen()` on the next line; merged declaration and initialization (×2 occurrences). `int c` (loop-local `fgetc()` result) is zero-initialized. - **`smokeyeyes.cpp`**: the `boost::program_options`-bound variables (`num_clients`, `turns`, `batch_size`, `no_wait`, `num_slots`) are populated by `po::store`/`po::notify` from their `default_value()`s; initialized to matching defaults so the variable is never observably uninitialized between declaration and parsing. `int wstatus`, an out-parameter for `waitpid()`, is zero-initialized. ## Verification - Scoped clang-tidy build isolated to `cppcoreguidelines-init-variables` shows **0 findings** (down from 11 across 4 files). - Full `bazel build //score/message_passing/... //score/mw/com/test/flock/... //score/mw/com/test/smokeyeyes/...` passes. - `bazel test //score/message_passing:unix_domain_test //score/message_passing:unix_domain_test_linux` passes (2/2). - `bazel test //:format_test_C++_with_clang-format` passes — no formatting issues introduced. ## Scope This PR intentionally contains **only** `cppcoreguidelines-init-variables` fixes, per the project convention of one clang-tidy check per PR (see eclipse-score#995, eclipse-score#999, eclipse-score#1000, eclipse-score#1001, eclipse-score#1003, eclipse-score#1012, eclipse-score#1013).
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 26, 2026
…indings (#1015) ## Summary Fixes all `cppcoreguidelines-special-member-functions` clang-tidy findings (3 findings across 3 files). ## Approach Explicitly declares the missing copy/move special member functions (deleted, since none of these classes support meaningful copy/move semantics) so each class fully follows the Rule of Five instead of relying on implicit generation, per the C++ Core Guidelines' recommendation that if any of destructor/copy/move is user-declared, all five should be declared. ## Fixes - **`sample_sender_receiver.cpp`**: `TestDestructor` is a scope-guard type that calls `stop_source_.request_stop()` in its destructor; it is only ever used as a named local variable and was never intended to be copied or moved. Deleted copy/move constructor and assignment. - **`provider.cpp` (partial_restart/provider_restart)**: `CyclicEventSender` already explicitly deleted its copy constructor/assignment (its `std::thread` member isn't copyable) but didn't declare move operations. Since its background thread captures `this`, moving the object would leave the thread referencing a stale instance, so move is deleted too, for the same reason copy already was. - **`smokeyeyes.cpp`**: `SharedState` owns a `pthread_barrier_t` that is placed directly into shared memory via placement-new and destroyed in its destructor; the barrier is tied to this object's address, so copying or moving it is not meaningful. Deleted copy/move constructor and assignment. All changes are pure declarations (`= delete`) with no behavioral change, since none of these types were ever copied or moved elsewhere in the codebase. ## Verification - Scoped clang-tidy build isolated to `cppcoreguidelines-special-member-functions` shows **0 findings** (down from 3 across 3 files). - Full `bazel build //score/mw/com/test/common_test_resources/... //score/mw/com/test/partial_restart/... //score/mw/com/test/smokeyeyes/...` passes. - `bazel test //score/mw/com/test/partial_restart/provider_restart/integration_test:provider_restart_graceful` passes. - `bazel test //:format_test_C++_with_clang-format` passes. ## Scope This PR intentionally contains **only** `cppcoreguidelines-special-member-functions` fixes, per the project convention of one clang-tidy check per PR (see #995, #999, #1000, #1001, #1003, #1012, #1013, #1014).
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.
Summary
Fixes all
misc-use-anonymous-namespaceclang-tidy findings (74 findings across 24 files) by manually applying the transformation the check recommends, since clang-tidy does not implement an automatic FixIt for this check.Why this couldn't be auto-fixed by the pipeline
I initially assumed the empty
--fixpatches produced byaspect_rules_lintwere a pipeline bug and spent time debugging it (verbose clang-tidy invocation, manual sandbox reproduction, etc.). Root cause: this is not a pipeline issue —misc-use-anonymous-namespaceonly emits a plain warning/note; it never attaches aFixItHint, confirmed by:--fixcommand (extracted from the actual Bazel sandboxed action) outside Bazel: exit 0, findings printed, but the file is byte-for-byte identical before/after.So the fixes below were applied by hand, following exactly what the check's message recommends ("move to anonymous namespace instead").
Changes
score/mw/com/doc/tutorial/chapter_{1,2,3,4,5,6,7,8,9,10,12}/{consumer,provider}): wrapped file-scopestaticglobals/functions (e.g.g_running,g_shutdown_mutex,SignalHandler,SendSample,UpdateField, ...) in an unnamed namespace and dropped the now-redundantstatickeyword.score/mw/com/impl/rust/com-api/com-api-ffi-lola/registry_bridge_macro.h: theBEGIN_EXPORT_MW_COM_INTERFACE/EXPORT_MW_COM_EVENTmacros generatestatic ..._reg_instance;declarations, which were the source of the remaining findings invehicle_gen.cppandbigdata_com_api_gen.cpp(the only two call sites of these macros in the repo). Fixed at the single root cause (the macro definitions) by wrapping the generated declarations in a nested anonymous namespace instead of touching the two generated call-site files.Verification
→ 0 remaining
misc-use-anonymous-namespacefindings in the affected scope.→ builds cleanly, including the Rust FFI bridge consumers of the modified macro.
Scope note
Per the earlier discussion, this PR is scoped only to
misc-use-anonymous-namespaceand does not touch thereadability-magic-numbers(#993), external-repo header-filter (#995), or magic-numbers-check-disable (#999) changes from the other PRs.