Skip to content

fix(clang-tidy): use UNREACHABLE for unreachable paths (RULE-0-0-2) - #1352

Merged
castler merged 2 commits into
eclipse-score:mainfrom
castler:fix_invariant_condition
Oct 16, 2026
Merged

fix(clang-tidy): use UNREACHABLE for unreachable paths (RULE-0-0-2)#1352
castler merged 2 commits into
eclipse-score:mainfrom
castler:fix_invariant_condition

Conversation

@castler

@castler castler commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Fixes CodeQL MISRA C++ RULE-0-0-2 (cpp/misra/invariant-condition) findings by replacing asserts on a constant condition (ASSERT_PRD(false), ASSERT_PRD(0), *_PRD_MESSAGE(false, ...)) with SCORE_LANGUAGE_FUTURECPP_UNREACHABLE[_MESSAGE].

Behaviour is unchanged: the _PRD asserts and UNREACHABLE both terminate in every assert level. Messages are kept.

Not covered (not asserts): operator< in service_instance_deployment.cpp and two sites in tracing_filter_config.cpp.

Validated: --config=ci build, //score/mw/com/impl/... and //score/memory/shared/... tests pass (244 pass, 3 skipped).

@castler
castler marked this pull request as ready for review October 9, 2026 08:46
@castler
castler enabled auto-merge October 9, 2026 08:46
bemerybmw
bemerybmw requested changes Oct 9, 2026
View reviewed changes
Comment thread score/memory/shared/pointer_arithmetic_util.cpp
<< "Could not subtract " << second_address_as_integer << "from" << first_address_as_integer
<< ". Result does not fit into std::ptrdiff_t. Terminating.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable? std::ptrdiff_t is signed while std::uintptr_t is unsigned. So you could theoretically have 2 pointers which are further than std::ptrdiff_t::max apart. I think that this is possible in code and also we should be able to write a test for this.

Comment thread score/memory/shared/pointer_arithmetic_util.cpp
<< first_address_as_integer
<< ". Result does not fit into std::ptrdiff_t. Terminating.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

Same as above

Comment thread score/mw/com/impl/bindings/lola/methods/offered_state_machine.cpp
OfferedStateMachine::State StopOfferedState::StopOffer()
{
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(0);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable? You can call StopOffer when in StopOfferedState

Comment thread score/mw/com/impl/bindings/lola/proxy.cpp Outdated
<< "Unable to find data storage for given event instance. Terminating.";
SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE(false,
"Unable to find data storage for given event instance.");
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("Unable to find data storage for given event instance.");

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable?

Comment thread score/mw/com/impl/bindings/lola/shm_path_builder.cpp
// default case and break are necessary to have well-formed switch.
// coverity[autosar_cpp14_m0_1_1_violation : FALSE]
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(false, "Invalid quality type");
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("Invalid quality type");

@bemerybmw bemerybmw Oct 9, 2026

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.

This is contradicting the comment above

Comment thread score/mw/com/impl/tracing/skeleton_event_tracing.cpp
// This is false positive, the enum has more fields than EVENT and FIELD so we might reach this branch.
// coverity[autosar_cpp14_m0_1_1_violation : FALSE]
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(false, "Service element type must be EVENT or FIELD");
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("Service element type must be EVENT or FIELD");

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable? Contradicts comment above

Comment thread score/mw/com/impl/tracing/skeleton_event_tracing.cpp
// This is false positive, the enum has more fields than EVENT and FIELD so we might reach this branch.
// coverity[autosar_cpp14_m0_1_1_violation : FALSE]
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(false, "Service element type must be EVENT or FIELD");
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE_MESSAGE("Service element type must be EVENT or FIELD");

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable? Contradicts comment above

Comment thread score/mw/com/impl/runtime.cpp
{
::score::mw::log::LogFatal("lola") << validation_result.error().UserMessage();
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

What is the Validate doing if it can never return an emply result?

Comment thread score/mw/com/impl/runtime.cpp
{
::score::mw::log::LogFatal("lola") << validation_result.error().UserMessage();
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

What is the Validate doing if it can never return an emply result?

Comment thread score/mw/com/impl/sample_allocatee_tracker.cpp
score::mw::log::LogFatal("lola") << "SampleAllocateeTracker destroyed while still holding" << num_allocated
<< "outstanding SampleAllocateePtr instance(s), terminating.";
SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD(false);
SCORE_LANGUAGE_FUTURECPP_UNREACHABLE();

@bemerybmw bemerybmw Oct 9, 2026

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.

Why is this unreachable?

bemerybmw
bemerybmw previously approved these changes Oct 16, 2026
View reviewed changes
@castler
castler force-pushed the fix_invariant_condition branch from 19b74de to 014ee2f Compare October 16, 2026 09:22
castler and others added 2 commits October 16, 2026 12:20
@castler
fix(clang-tidy): use SCORE_LANGUAGE_FUTURECPP_UNREACHABLE for unreach…
dd21032 
…able paths

Replace ASSERT_PRD(false), ASSERT_PRD(0) and the *_PRD_MESSAGE(false, ...)
variants with SCORE_LANGUAGE_FUTURECPP_UNREACHABLE[_MESSAGE]. They express
that the path must never be taken and always terminate, like the _PRD
variants, without an assertion on a constant condition. Fixes the CodeQL
RULE-0-0-2 (invariant condition) findings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@castler
docs(clang-tidy): document why each UNREACHABLE path is reachable but…
014ee2f 
… not expected

Add a comment above each SCORE_LANGUAGE_FUTURECPP_UNREACHABLE[_MESSAGE]
introduced for RULE-0-0-2 stating that the path is reachable in principle,
why, and that it must not be reached in production.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@castler
castler added this pull request to the merge queue Oct 16, 2026
Merged via the queue into eclipse-score:main with commit ffbf5ac Oct 16, 2026
18 of 27 checks passed
@castler
castler deleted the fix_invariant_condition branch October 16, 2026 16:52
@github-project-automation github-project-automation Bot moved this from Backlog to Done in COM - Communication FT Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bemerybmw bemerybmw bemerybmw approved these changes
@hoe-jo hoe-jo Awaiting requested review from hoe-jo hoe-jo is a code owner
@LittleHuba LittleHuba Awaiting requested review from LittleHuba LittleHuba is a code owner
@crimson11 crimson11 Awaiting requested review from crimson11 crimson11 is a code owner
@limdor limdor Awaiting requested review from limdor limdor is a code owner

Assignees

No one assigned

Labels

None yet

Projects

COM - Communication FT
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@castler @bemerybmw