fix: fix if-init done by AI by asalzburger · Pull Request #5312 · acts-project/acts

asalzburger · 2026-04-03T04:42:48Z

C++17 `if` init-statement (Sonar)

***These are fixes done by cursor AI running on (part of) our SonarCloud output ***

Summary

This change addresses Sonar findings that ask to declare a variable in the init-statement of an if (C++17), instead of declaring it on the preceding line and only using it in the condition and immediate body. The refactor narrows scope, matches modern C++ style, and clears the corresponding “Use the init-statement to declare … inside the if statement” issues (76 in the exported Sonar report used for triage).

Pattern

Before:

auto x = f();
if (!x.ok()) {
  return x.error();
}

After:

if (auto x = f(); !x.ok()) {
  return x.error();
}

The same idea applies to pointers, booleans, iterator results, Result types, dynamic_cast results, and similar “compute once, then branch” code.

Notable cases

Whole if / else if chains: Names introduced in the init-statement are in scope for the entire if / else if / else chain. This is used for flags such as typeFlags and currentState where several branches need the same value.
CompositeSpacePointLineFitter: precResult is initialized in the first if; the following else if (precResult) remains valid because it is part of the same if statement.
GaussianSumFitter: One branch uses if constexpr (constexpr bool IsMultiParameters = …; !IsMultiParameters) (C++20), consistent with the project standard.
VectorHelpers::perp: if constexpr with an init-statement for RowsAtCompileTime.
SteppingHelper: farLimit is only needed for the reachability check; it is moved into that if’s init-statement while nearLimit stays outside.

Files touched (39)

Area	Path
Alignment	`Alignment/include/ActsAlignment/Kernel/Alignment.ipp`
Ambiguity	`Core/include/Acts/AmbiguityResolution/ScoreBasedAmbiguityResolution.ipp`
Event data	`Core/include/Acts/EventData/MultiTrajectoryHelpers.hpp`
Propagator	`Core/include/Acts/Propagator/DirectNavigator.hpp`
	`Core/include/Acts/Propagator/EigenStepper.ipp`
	`Core/include/Acts/Propagator/MaterialInteractor.hpp`
	`Core/include/Acts/Propagator/MultiStepperLoop.ipp`
	`Core/include/Acts/Propagator/Propagator.ipp`
	`Core/include/Acts/Propagator/StandardAborters.hpp`
	`Core/include/Acts/Propagator/SurfaceCollector.hpp`
	`Core/include/Acts/Propagator/VolumeCollector.hpp`
	`Core/include/Acts/Propagator/detail/SteppingHelper.hpp`
Seeding	`Core/include/Acts/Seeding/BinnedGroupIterator.ipp`
	`Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp`
Track finding	`Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp`
	`Core/include/Acts/TrackFinding/TrackStateCreator.hpp`
Track fitting	`Core/include/Acts/TrackFitting/GaussianSumFitter.hpp`
	`Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp`
	`Core/include/Acts/TrackFitting/KalmanFitter.hpp`
	`Core/include/Acts/TrackFitting/detail/GsfActor.hpp`
Utilities	`Core/include/Acts/Utilities/AlgebraHelpers.hpp`
	`Core/include/Acts/Utilities/Frustum.ipp`
	`Core/include/Acts/Utilities/TrackHelpers.hpp`
	`Core/include/Acts/Utilities/TypeDispatcher.hpp`
	`Core/include/Acts/Utilities/UnitVectors.hpp`
	`Core/include/Acts/Utilities/VectorHelpers.hpp`
Core sources	`Core/src/EventData/CorrectedTransformationFreeToBound.cpp`
Geometry	`Core/src/Geometry/ConeVolumeBounds.cpp`
	`Core/src/Geometry/CuboidVolumeStack.cpp`
	`Core/src/Geometry/CutoutCylinderVolumeBounds.cpp`
	`Core/src/Geometry/CylinderPortalShell.cpp`
	`Core/src/Geometry/CylinderVolumeBuilder.cpp`
	`Core/src/Geometry/CylinderVolumeHelper.cpp`
	`Core/src/Geometry/CylinderVolumeStack.cpp`
	`Core/src/Geometry/DiscLayer.cpp`
	`Core/src/Geometry/GlueVolumesDescriptor.cpp`
	`Core/src/Geometry/LayerArrayCreator.cpp`
	`Core/src/Geometry/MultiWireVolumeBuilder.cpp`

Verification

Build: acts build acts-sonarcloud (from ~/Documents/work/dev with your usual acts environment).
Tests: acts test acts-sonarcloud (full ctest suite for the project).

No intended behavioral change: only scope and style, except where the previous variable would have leaked into a wider block (now correctly limited to the if statement).

--- END COMMIT MESSAGE ---

Any further description goes here, @-mentions are ok here!

Use a conventional commits prefix: quick summary
- We mostly use feat, fix, refactor, docs, chore and build types.
A milestone will be assigned by one of the maintainers

github-actions · 2026-04-03T06:22:48Z

📊: Physics performance monitoring for `b6183fd`

Full contents

physmon summary

andiwand · 2026-04-03T09:13:06Z

Alignment/include/ActsAlignment/Kernel/Alignment.ipp

    // Set the alignment mask
-    auto iter_it = alignOptions.iterationState.find(iIter);
-    if (iter_it != alignOptions.iterationState.end()) {
+    if (auto iter_it = alignOptions.iterationState.find(iIter);


Suggested change

if (auto iter_it = alignOptions.iterationState.find(iIter);

if (const auto iter_it = alignOptions.iterationState.find(iIter);

andiwand · 2026-04-03T09:13:13Z

Alignment/include/ActsAlignment/Kernel/Alignment.ipp

-        alignOptions.fitOptions.geoContext, alignOptions.alignedDetElements,
-        alignOptions.alignedTransformUpdater, alignResult);
-    if (!updateRes.ok()) {
+    if (auto updateRes = updateAlignmentParameters(


Suggested change

if (auto updateRes = updateAlignmentParameters(

if (const auto updateRes = updateAlignmentParameters(

andiwand · 2026-04-03T09:13:35Z

Core/include/Acts/AmbiguityResolution/ScoreBasedAmbiguityResolution.ipp

-      auto isoutliner = ts.typeFlags().isOutlier();
-
-      if (isoutliner) {
+      if (auto isoutliner = ts.typeFlags().isOutlier(); isoutliner) {


Suggested change

if (auto isoutliner = ts.typeFlags().isOutlier(); isoutliner) {

if (const bool isOutliner = ts.typeFlags().isOutlier(); isOutliner) {

andiwand · 2026-04-03T09:13:44Z

Core/include/Acts/EventData/MultiTrajectoryHelpers.hpp

    trajState.nStates++;
-    auto typeFlags = state.typeFlags();
-    if (typeFlags.isHole()) {
+    if (auto typeFlags = state.typeFlags(); typeFlags.isHole()) {


Suggested change

if (auto typeFlags = state.typeFlags(); typeFlags.isHole()) {

if (const auto typeFlags = state.typeFlags(); typeFlags.isHole()) {

andiwand · 2026-04-03T09:13:51Z

Core/include/Acts/EventData/MultiTrajectoryHelpers.hpp

    trajState.NDF += state.calibratedSize();
-    auto typeFlags = state.typeFlags();
-    if (typeFlags.isHole()) {
+    if (auto typeFlags = state.typeFlags(); typeFlags.isHole()) {


Suggested change

if (auto typeFlags = state.typeFlags(); typeFlags.isHole()) {

if (const auto typeFlags = state.typeFlags(); typeFlags.isHole()) {

andiwand · 2026-04-03T09:18:37Z

Core/include/Acts/Propagator/SurfaceCollector.hpp

+    if (auto currentSurface = navigator.currentSurface(state.navigation);
+        currentSurface && selector(*currentSurface)) {


Suggested change

if (auto currentSurface = navigator.currentSurface(state.navigation);

currentSurface && selector(*currentSurface)) {

if (const Surface* currentSurface = navigator.currentSurface(state.navigation);

currentSurface != nullptr && selector(*currentSurface)) {

andiwand · 2026-04-03T09:18:55Z

Core/include/Acts/Propagator/VolumeCollector.hpp

+    if (auto currentVolume = navigator.currentVolume(state.navigation);
+        currentVolume && selector(*currentVolume)) {


Suggested change

if (auto currentVolume = navigator.currentVolume(state.navigation);

currentVolume && selector(*currentVolume)) {

if (const Volume* currentVolume = navigator.currentVolume(state.navigation);

currentVolume != nullptr && selector(*currentVolume)) {

Core/include/Acts/Seeding/BinnedGroupIterator.ipp

andiwand · 2026-04-03T09:19:52Z

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp

+  if (precResult_t precResult =
+          doPrecFit ? fastPrecFit<fitStraws, fitTime>(
+                          measurements, initialGuess, precFitDelegate)
+                    : std::nullopt;
+      doPrecFit && !precResult) {


looks more involved now

Previous is better, readability is worse now and an if clause within an if initialization doesn't really help.

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp

andiwand · 2026-04-03T10:50:14Z

Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp

here I would also argue that it hurts readability

Core/include/Acts/TrackFinding/TrackStateCreator.hpp

andiwand · 2026-04-03T10:56:04Z

Core/include/Acts/TrackFitting/detail/GsfActor.hpp

also here I fear it hurts the algorithm flow

andiwand · 2026-04-03T11:01:24Z

Core/src/Geometry/CylinderPortalShell.cpp

false positive IMO

andiwand · 2026-04-03T11:02:43Z

Core/src/Geometry/GlueVolumesDescriptor.cpp

  // register the face
-  auto searchIter = m_glueVolumes.find(bsf);
-  if (searchIter == m_glueVolumes.end()) {
+  if (auto searchIter = m_glueVolumes.find(bsf);


Suggested change

if (auto searchIter = m_glueVolumes.find(bsf);

if (const auto searchIter = m_glueVolumes.find(bsf);

andiwand · 2026-04-03T11:02:50Z

Core/src/Geometry/GlueVolumesDescriptor.cpp

  // searching for the glue volumes according
-  auto searchIter = m_glueVolumes.find(bsf);
-  if (searchIter != m_glueVolumes.end()) {
+  if (auto searchIter = m_glueVolumes.find(bsf);


Suggested change

if (auto searchIter = m_glueVolumes.find(bsf);

if (const auto searchIter = m_glueVolumes.find(bsf);

andiwand · 2026-04-03T11:03:20Z

Core/src/Geometry/MultiWireVolumeBuilder.cpp

+  if (auto boundsType = m_config.bounds ? m_config.bounds->type()
+                                        : VolumeBounds::BoundsType::eOther;


Suggested change

if (auto boundsType = m_config.bounds ? m_config.bounds->type()

: VolumeBounds::BoundsType::eOther;

if (const VolumeBounds::BoundsType boundsType = m_config.bounds ? m_config.bounds->type()

: VolumeBounds::BoundsType::eOther;

andiwand · 2026-04-03T11:03:46Z

Core/src/Geometry/MultiWireVolumeBuilder.cpp

+      !(boundsType == VolumeBounds::BoundsType::eTrapezoid ||
        boundsType == VolumeBounds::BoundsType::eCuboid ||
        boundsType == VolumeBounds::BoundsType::eDiamond)) {


a local using enum could improve readability here

I had tried another one for the using enum but I need to narrow that down to case-by-case.

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

asalzburger

First pass through, no reverts yet.

asalzburger · 2026-04-05T05:40:36Z

Core/include/Acts/Propagator/detail/SteppingHelper.hpp

  const double nearLimit = std::numeric_limits<double>::lowest();
-  const double farLimit = std::numeric_limits<double>::max();

-  if (sIntersection.isValid() &&
+  if (const double farLimit = std::numeric_limits<double>::max();
+      sIntersection.isValid() &&


I agree, makes it better readable.

asalzburger · 2026-04-05T05:41:13Z

Core/include/Acts/Propagator/DirectNavigator.hpp

+      if (const NavigationTarget target = chooseIntersection(
+              state.options.geoContext, surface, position, direction,
+              BoundaryTolerance::Infinite(), state.options.nearLimit, farLimit,
+              state.options.surfaceTolerance);
+          target.isValid()) {
        return target;
      }


Point taken.

asalzburger · 2026-04-05T05:41:40Z

Core/include/Acts/Propagator/EigenStepper.ipp

+  if (const double initialStepLength = std::abs(initialH);
+      nextAccuracy < initialStepLength || nextAccuracy > previousAccuracy) {
    state.stepSize.setAccuracy(nextAccuracy);


asalzburger · 2026-04-05T05:43:04Z

Core/include/Acts/Propagator/MultiStepperLoop.ipp

+  if (const auto cmpsOnSurface =
+          std::count_if(components.cbegin(), components.cend(),
+                        [&](auto& cmp) {
+                          return cmp.status == IntersectionStatus::onSurface;
+                        });
+      cmpsOnSurface > 0) {


Previous is better.

Core/include/Acts/Propagator/StandardAborters.hpp

asalzburger · 2026-04-05T05:46:15Z

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp

+  if (precResult_t precResult =
+          doPrecFit ? fastPrecFit<fitStraws, fitTime>(
+                          measurements, initialGuess, precFitDelegate)
+                    : std::nullopt;
+      doPrecFit && !precResult) {


Previous is better, readability is worse now and an if clause within an if initialization doesn't really help.

Core/include/Acts/Seeding/CompositeSpacePointLineFitter.ipp

Core/include/Acts/TrackFinding/TrackStateCreator.hpp

asalzburger · 2026-04-05T05:52:04Z

Core/src/Geometry/MultiWireVolumeBuilder.cpp

+      !(boundsType == VolumeBounds::BoundsType::eTrapezoid ||
        boundsType == VolumeBounds::BoundsType::eCuboid ||
        boundsType == VolumeBounds::BoundsType::eDiamond)) {


I had tried another one for the using enum but I need to narrow that down to case-by-case.

sonarqubecloud · 2026-04-05T07:33:18Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
4.5% Duplication on New Code

See analysis details on SonarQube Cloud

asalzburger · 2026-04-07T06:58:16Z

I am gonna close this one, and try to refine the prompt.

fix if initial

b6183fd

github-actions bot added Component - Core Affects the Core module Event Data Model Seeding Track Finding Track Fitting Ambiguity Resolution labels Apr 3, 2026

github-actions bot added this to the next milestone Apr 3, 2026

andiwand mentioned this pull request Apr 3, 2026

fix: std::ranges fixes done by AI #5311

Merged

2 tasks

andiwand reviewed Apr 3, 2026

View reviewed changes

asalzburger and others added 3 commits April 5, 2026 07:38

adressing first part of PR comments

ff4805c

Apply suggestion from @andiwand

68986ec

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

Apply suggestions from code review

0a59935

Co-authored-by: Andreas Stefl <stefl.andreas@gmail.com>

asalzburger commented Apr 5, 2026

View reviewed changes

asalzburger marked this pull request as draft April 5, 2026 07:03

asalzburger closed this Apr 7, 2026

andiwand modified the milestones: next, v46.2.0 Apr 14, 2026

	if (auto iter_it = alignOptions.iterationState.find(iIter);
	if (const auto iter_it = alignOptions.iterationState.find(iIter);

	if (auto updateRes = updateAlignmentParameters(
	if (const auto updateRes = updateAlignmentParameters(

	if (auto isoutliner = ts.typeFlags().isOutlier(); isoutliner) {
	if (const bool isOutliner = ts.typeFlags().isOutlier(); isOutliner) {

	if (auto typeFlags = state.typeFlags(); typeFlags.isHole()) {
	if (const auto typeFlags = state.typeFlags(); typeFlags.isHole()) {

		if (auto currentSurface = navigator.currentSurface(state.navigation);
		currentSurface && selector(*currentSurface)) {

		if (auto currentVolume = navigator.currentVolume(state.navigation);
		currentVolume && selector(*currentVolume)) {

	if (auto searchIter = m_glueVolumes.find(bsf);
	if (const auto searchIter = m_glueVolumes.find(bsf);

		if (auto boundsType = m_config.bounds ? m_config.bounds->type()
		: VolumeBounds::BoundsType::eOther;

Conversation

asalzburger commented Apr 3, 2026

C++17 if init-statement (Sonar)

Summary

Pattern

Notable cases

Files touched (39)

Verification

Uh oh!

github-actions bot commented Apr 3, 2026

📊: Physics performance monitoring for b6183fd

physmon summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asalzburger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 5, 2026

Quality Gate passed

Uh oh!

asalzburger commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

C++17 `if` init-statement (Sonar)

📊: Physics performance monitoring for `b6183fd`