chore: clang-tidy performance-move-const-arg (#1359)

This enables the `performance-move-const-arg` check in clang-tidy.
It suggests replacing/removing `std::move` where it won't actually do anything, e.g. because the called function takes a const reference anyway, or the underlying type is trivially copyable. This PR removes the `std::move` calls in these cases.

.clang-tidy

@@ -1,4 +1,4 @@
 ---
-Checks: '-*,readability-container-size-empty,readability-implicit-bool-cast,readability-implicit-bool-conversion,modernize-use-equals-default,modernize-use-override,modernize-use-using,readability-braces-around-statements'
+Checks: '-*,readability-container-size-empty,readability-implicit-bool-cast,readability-implicit-bool-conversion,modernize-use-equals-default,modernize-use-override,modernize-use-using,readability-braces-around-statements,performance-move-const-arg'
 HeaderFilterRegex: '.*(?<!nlohmann\/json)\.(hpp|cpp|ipp)$'
 AnalyzeTemporaryDtors: true

CI/clang_tidy/limits.yml

@@ -12,3 +12,4 @@ limits:
   # "performance-unnecessary-value-param": 0
   # "modernize-use-equals-default": 0
   # "modernize-use-nullptr": 0
+  "performance-move-const-arg": 0

Core/include/Acts/Surfaces/ConeSurface.hpp

@@ -56,7 +56,7 @@ class ConeSurface : public Surface {
   /// @param transform is the transform that places the cone in the global frame
   /// @param cbounds is the boundary class, the bounds must exit
   ConeSurface(const Transform3& transform,
-              const std::shared_ptr<const ConeBounds>& cbounds);
+              std::shared_ptr<const ConeBounds> cbounds);
 
   /// Copy constructor
   ///

Core/src/MagneticField/SolenoidBField.cpp

@@ -16,7 +16,7 @@
 #include <boost/math/special_functions/ellint_1.hpp>
 #include <boost/math/special_functions/ellint_2.hpp>
 
-Acts::SolenoidBField::SolenoidBField(Config config) : m_cfg(std::move(config)) {
+Acts::SolenoidBField::SolenoidBField(Config config) : m_cfg(config) {
   m_dz = m_cfg.length / m_cfg.nCoils;
   m_R2 = m_cfg.radius * m_cfg.radius;
   // we need to scale so we reproduce the expected B field strength

Core/src/Material/MaterialGridHelper.cpp

@@ -164,7 +164,7 @@ Acts::Grid2D Acts::createGrid2D(
     pos = transfo * pos;
     return {coord1(pos), coord2(pos)};
   };
-  return (Acts::createGrid(std::move(gridAxis1), std::move(gridAxis2)));
+  return (Acts::createGrid(gridAxis1, gridAxis2));
 }
 
 Acts::Grid3D Acts::createGrid3D(
@@ -216,8 +216,7 @@ Acts::Grid3D Acts::createGrid3D(
     pos = transfo * pos;
     return {coord1(pos), coord2(pos), coord3(pos)};
   };
-  return (Acts::createGrid(std::move(gridAxis1), std::move(gridAxis2),
-                           std::move(gridAxis3)));
+  return (Acts::createGrid(gridAxis1, gridAxis2, gridAxis3));
 }
 
 Acts::MaterialGrid2D Acts::mapMaterialPoints(Acts::Grid2D& grid) {

Core/src/Material/VolumeMaterialMapper.cpp

@@ -315,7 +315,7 @@ void Acts::VolumeMaterialMapper::finalizeMaps(State& mState) const {
       ACTS_DEBUG("Homogeneous material volume");
       Acts::Material mat = mState.homogeneousGrid[matBin.first].average();
       mState.volumeMaterial[matBin.first] =
-          std::make_unique<HomogeneousVolumeMaterial>(std::move(mat));
+          std::make_unique<HomogeneousVolumeMaterial>(mat);
     } else if (matBin.second.dimensions() == 2) {
       // Average the material in the 2D grid then create an
       // InterpolatedMaterialMap

Core/src/Surfaces/ConeSurface.cpp

@@ -43,9 +43,9 @@ Acts::ConeSurface::ConeSurface(const Transform3& transform, double alpha,
 }
 
 Acts::ConeSurface::ConeSurface(const Transform3& transform,
-                               const std::shared_ptr<const ConeBounds>& cbounds)
-    : GeometryObject(), Surface(transform), m_bounds(cbounds) {
-  throw_assert(cbounds, "ConeBounds must not be nullptr");
+                               std::shared_ptr<const ConeBounds> cbounds)
+    : GeometryObject(), Surface(transform), m_bounds(std::move(cbounds)) {
+  throw_assert(m_bounds, "ConeBounds must not be nullptr");
 }
 
 Acts::Vector3 Acts::ConeSurface::binningPosition(
@@ -76,7 +76,7 @@ Acts::ConeSurface& Acts::ConeSurface::operator=(const ConeSurface& other) {
 
 Acts::Vector3 Acts::ConeSurface::rotSymmetryAxis(
     const GeometryContext& gctx) const {
-  return std::move(transform(gctx).matrix().block<3, 1>(0, 2));
+  return transform(gctx).matrix().block<3, 1>(0, 2);
 }
 
 Acts::RotationMatrix3 Acts::ConeSurface::referenceFrame(
@@ -408,4 +408,4 @@ Acts::ActsMatrix<2, 3> Acts::ConeSurface::localCartesianToBoundLocalDerivative(
       lphi * bounds().tanAlpha(), 0, 0, 1;
 
   return loc3DToLocBound;
-}
+}

Core/src/Surfaces/DiscSurface.cpp

@@ -36,7 +36,7 @@ Acts::DiscSurface::DiscSurface(const GeometryContext& gctx,
 Acts::DiscSurface::DiscSurface(const Transform3& transform, double rmin,
                                double rmax, double hphisec)
     : GeometryObject(),
-      Surface(std::move(transform)),
+      Surface(transform),
       m_bounds(std::make_shared<const RadialBounds>(rmin, rmax, hphisec)) {}
 
 Acts::DiscSurface::DiscSurface(const Transform3& transform, double minhalfx,

Examples/Algorithms/Propagation/src/PropagationAlgorithm.cpp

@@ -69,16 +69,15 @@ ProcessCode PropagationAlgorithm::execute(
     PropagationOutput pOutput;
     if (charge != 0.0) {
       // charged extrapolation - with hit recording
-      Acts::BoundTrackParameters startParameters(surface, std::move(pars),
-                                                 std::move(cov));
+      Acts::BoundTrackParameters startParameters(surface, pars, std::move(cov));
       sPosition = startParameters.position(context.geoContext);
       sMomentum = startParameters.momentum();
       pOutput = m_cfg.propagatorImpl->execute(
           context, m_cfg, Acts::LoggerWrapper{logger()}, startParameters);
     } else {
       // execute the test for neutral particles
-      Acts::NeutralBoundTrackParameters neutralParameters(
-          surface, std::move(pars), std::move(cov));
+      Acts::NeutralBoundTrackParameters neutralParameters(surface, pars,
+                                                          std::move(cov));
       sPosition = neutralParameters.position(context.geoContext);
       sMomentum = neutralParameters.momentum();
       pOutput = m_cfg.propagatorImpl->execute(

Examples/Algorithms/TrackFinding/src/TrackFindingAlgorithm.cpp

@@ -100,10 +100,9 @@ ActsExamples::ProcessCode ActsExamples::TrackFindingAlgorithm::execute(
       // Get the track finding output object
       const auto& trackFindingOutput = result.value();
       // Create a Trajectories result struct
-      trajectories.emplace_back(
-          std::move(trackFindingOutput.fittedStates),
-          std::move(trackFindingOutput.lastMeasurementIndices),
-          std::move(trackFindingOutput.fittedParameters));
+      trajectories.emplace_back(trackFindingOutput.fittedStates,
+                                trackFindingOutput.lastMeasurementIndices,
+                                trackFindingOutput.fittedParameters);
     } else {
       ACTS_WARNING("Track finding failed for seed " << iseed << " with error"
                                                     << result.error());

Examples/Algorithms/TrackFitting/src/TrackFittingAlgorithm.cpp

@@ -142,14 +142,13 @@ ActsExamples::ProcessCode ActsExamples::TrackFittingAlgorithm::execute(
         ACTS_VERBOSE("Fitted paramemeters for track " << itrack);
         ACTS_VERBOSE("  " << params.parameters().transpose());
         // Push the fitted parameters to the container
-        indexedParams.emplace(fitOutput.lastMeasurementIndex,
-                              std::move(params));
+        indexedParams.emplace(fitOutput.lastMeasurementIndex, params);
       } else {
         ACTS_DEBUG("No fitted paramemeters for track " << itrack);
       }
       // store the result
-      trajectories.emplace_back(std::move(fitOutput.fittedStates),
-                                std::move(trackTips), std::move(indexedParams));
+      trajectories.emplace_back(fitOutput.fittedStates, std::move(trackTips),
+                                std::move(indexedParams));
     } else {
       ACTS_WARNING("Fit failed for track "
                    << itrack << " with error: " << result.error() << ", "

Examples/Algorithms/TrackFitting/src/TrackFittingAlgorithmFunctionsGsf.cpp

@@ -136,7 +136,7 @@ TrackFittingAlgorithm::makeGsfFitterFunction(
     bool disableAllMaterialHandling) {
   Acts::MultiEigenStepperLoop stepper(std::move(magneticField));
   Acts::DirectNavigator navigator;
-  Acts::Propagator propagator(std::move(stepper), std::move(navigator));
+  Acts::Propagator propagator(std::move(stepper), navigator);
   Acts::GaussianSumFitter<decltype(propagator)> trackFitter(
       std::move(propagator));
 

Examples/Algorithms/TrackFitting/src/TrackFittingAlgorithmFunctionsKalman.cpp

@@ -163,7 +163,7 @@ ActsExamples::TrackFittingAlgorithm::makeKalmanFitterFunction(
   // construct all components for the fitter
   Stepper stepper(std::move(magneticField));
   Acts::DirectNavigator navigator;
-  DirectPropagator propagator(std::move(stepper), std::move(navigator));
+  DirectPropagator propagator(std::move(stepper), navigator);
   DirectFitter fitter(std::move(propagator));
 
   // build the fitter functions. owns the fitter object.

Examples/Algorithms/Vertexing/src/AdaptiveMultiVertexFinderAlgorithm.cpp

@@ -156,7 +156,8 @@ ActsExamples::AdaptiveMultiVertexFinderAlgorithm::execute(
   int timeMS =
       std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();
   // store reconstruction time
-  ctx.eventStore.add(m_cfg.outputTime, std::move(timeMS));
+  ctx.eventStore.add(m_cfg.outputTime,
+                     std::move(timeMS));  // NOLINT(performance-move-const-arg)
 
   return ActsExamples::ProcessCode::SUCCESS;
 }

Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp

@@ -83,18 +83,18 @@ ActsExamples::ProcessCode ActsExamples::IterativeVertexFinderAlgorithm::execute(
                                    Acts::LoggerWrapper{logger()});
   // Setup the vertex fitter
   VertexFitter::Config vertexFitterCfg;
-  VertexFitter vertexFitter(std::move(vertexFitterCfg));
+  VertexFitter vertexFitter(vertexFitterCfg);
   // Setup the track linearizer
   Linearizer::Config linearizerCfg(m_cfg.bField, propagator);
-  Linearizer linearizer(std::move(linearizerCfg));
+  Linearizer linearizer(linearizerCfg);
   // Setup the seed finder
   ImpactPointEstimator::Config ipEstCfg(m_cfg.bField, propagator);
-  ImpactPointEstimator ipEst(std::move(ipEstCfg));
+  ImpactPointEstimator ipEst(ipEstCfg);
   VertexSeeder::Config seederCfg(ipEst);
-  VertexSeeder seeder(std::move(seederCfg));
+  VertexSeeder seeder(seederCfg);
   // Set up the actual vertex finder
-  VertexFinder::Config finderCfg(std::move(vertexFitter), std::move(linearizer),
-                                 std::move(seeder), ipEst);
+  VertexFinder::Config finderCfg(vertexFitter, linearizer, std::move(seeder),
+                                 ipEst);
   finderCfg.maxVertices = 200;
   finderCfg.reassignTracksAfterFirstFit = true;
   VertexFinder finder(finderCfg);
@@ -130,7 +130,8 @@ ActsExamples::ProcessCode ActsExamples::IterativeVertexFinderAlgorithm::execute(
   int timeMS =
       std::chrono::duration_cast<std::chrono::milliseconds>(t2 - t1).count();
   // store reconstruction time
-  ctx.eventStore.add(m_cfg.outputTime, std::move(timeMS));
+  ctx.eventStore.add(m_cfg.outputTime,
+                     std::move(timeMS));  // NOLINT(performance-move-const-arg)
 
   return ActsExamples::ProcessCode::SUCCESS;
 }

Examples/Detectors/GenericDetector/src/GenericDetectorElement.cpp

@@ -20,7 +20,7 @@ ActsExamples::Generic::GenericDetectorElement::GenericDetectorElement(
     std::shared_ptr<const Acts::ISurfaceMaterial> material,
     std::shared_ptr<const Acts::DigitizationModule> digitizationModule)
     : Acts::IdentifiedDetectorElement(),
-      m_elementIdentifier(std::move(identifier)),
+      m_elementIdentifier(identifier),
       m_elementTransform(std::move(transform)),
       m_elementSurface(
           Acts::Surface::makeShared<Acts::PlaneSurface>(pBounds, *this)),
@@ -40,7 +40,7 @@ ActsExamples::Generic::GenericDetectorElement::GenericDetectorElement(
     std::shared_ptr<const Acts::ISurfaceMaterial> material,
     std::shared_ptr<const Acts::DigitizationModule> digitizationModule)
     : Acts::IdentifiedDetectorElement(),
-      m_elementIdentifier(std::move(identifier)),
+      m_elementIdentifier(identifier),
       m_elementTransform(std::move(transform)),
       m_elementSurface(
           Acts::Surface::makeShared<Acts::DiscSurface>(dBounds, *this)),

Examples/Framework/src/Framework/Sequencer.cpp

@@ -215,7 +215,7 @@ struct TimingInfo {
 void storeTiming(const std::vector<std::string>& identifiers,
                  const std::vector<Duration>& durations, std::size_t numEvents,
                  std::string path) {
-  dfe::NamedTupleTsvWriter<TimingInfo> writer(std::move(path), 4);
+  dfe::NamedTupleTsvWriter<TimingInfo> writer(path, 4);
   for (size_t i = 0; i < identifiers.size(); ++i) {
     TimingInfo info;
     info.identifier = identifiers[i];

Examples/Python/src/Geant4Component.cpp

@@ -180,7 +180,7 @@ PYBIND11_MODULE(ActsPythonBindingsGeant4, mod) {
         // Set the primarty generator
         g4Cfg.primaryGeneratorAction = new SimParticleTranslation(
             g4PrCfg, Acts::getDefaultLogger("SimParticleTranslation", level));
-        g4Cfg.detectorConstruction = std::move(detector);
+        g4Cfg.detectorConstruction = detector;
 
         // Set the user actions
         g4Cfg.runActions = runActions;

Examples/Run/Common/src/CommonMaterialMapping.cpp

@@ -122,7 +122,7 @@ int runMaterialMapping(int argc, char* argv[],
     Acts::Navigator navigator({tGeometry, true, true, true});
     // Make stepper and propagator
     SlStepper stepper;
-    Propagator propagator(std::move(stepper), std::move(navigator));
+    Propagator propagator(stepper, std::move(navigator));
     /// The material surface mapper
     Acts::SurfaceMaterialMapper::Config smmConfig;
     auto smm = std::make_shared<Acts::SurfaceMaterialMapper>(
@@ -135,7 +135,7 @@ int runMaterialMapping(int argc, char* argv[],
     Acts::Navigator navigator({tGeometry});
     // Make stepper and propagator
     SlStepper stepper;
-    Propagator propagator(std::move(stepper), std::move(navigator));
+    Propagator propagator(stepper, std::move(navigator));
     /// The material volume mapper
     Acts::VolumeMaterialMapper::Config vmmConfig;
     vmmConfig.mappingStep = volumeStep;

Examples/Run/Common/src/GeometryExampleBase.cpp

@@ -180,8 +180,7 @@ int processGeometry(int argc, char* argv[],
       }
       jmWriterCfg.writeFormat = format;
 
-      ActsExamples::JsonMaterialWriter jmwImpl(std::move(jmWriterCfg),
-                                               logLevel);
+      ActsExamples::JsonMaterialWriter jmwImpl(jmWriterCfg, logLevel);
 
       jmwImpl.write(*tGeometry);
     }

Examples/Run/Common/src/MaterialValidationBase.cpp

@@ -111,7 +111,7 @@ ActsExamples::ProcessCode setupStraightLinePropagation(
   using Propagator = Acts::Propagator<SlStepper, Acts::Navigator>;
   // Make stepper and propagator
   SlStepper stepper;
-  Propagator propagator(std::move(stepper), std::move(navigator));
+  Propagator propagator(stepper, std::move(navigator));
 
   // Read the propagation config and create the algorithms
   auto pAlgConfig = ActsExamples::Options::readPropagationConfig(vm);

Examples/Run/HelloWorld/HelloService.cpp

@@ -25,7 +25,9 @@ void HelloService::prepare(AlgorithmContext& ctx) {
   // integer division should round down
   std::size_t blockIndex = ctx.eventNumber / m_cfg.eventsPerBlock;
   // add block index to the event store
-  ctx.eventStore.add(m_cfg.blockIndexName, std::move(blockIndex));
+  ctx.eventStore.add(
+      m_cfg.blockIndexName,
+      std::move(blockIndex));  // NOLINT(performance-move-const-arg)
 }
 
 }  // namespace ActsExamples

Plugins/DD4hep/src/DD4hepDetectorElement.cpp

@@ -23,4 +23,4 @@ Acts::DD4hepDetectorElement::DD4hepDetectorElement(
                                 *(detElement.placement().ptr()),
                                 detElement.nominal().worldTransformation(),
                                 axes, scalor, std::move(material)),
-      m_detElement(std::move(detElement)) {}
+      m_detElement(detElement) {}

Plugins/Json/src/MaterialMapJsonConverter.cpp

@@ -178,7 +178,7 @@ Acts::TrackingVolumeAndMaterial defaultVolumeMaterial(
 Acts::MaterialMapJsonConverter::MaterialMapJsonConverter(
     const Acts::MaterialMapJsonConverter::Config& config,
     Acts::Logging::Level level)
-    : m_cfg(std::move(config)),
+    : m_cfg(config),
       m_logger{getDefaultLogger("MaterialMapJsonConverter", level)},
       m_volumeMaterialConverter(m_volumeName),
       m_volumeConverter(m_volumeName),

Tests/UnitTests/Alignment/Kernel/AlignmentTests.cpp

@@ -182,7 +182,7 @@ StraightPropagator makeStraightPropagator(
   cfg.resolveSensitive = true;
   Navigator navigator(cfg);
   StraightLineStepper stepper;
-  return StraightPropagator(std::move(stepper), std::move(navigator));
+  return StraightPropagator(stepper, std::move(navigator));
 }
 
 // Construct a propagator using a constant magnetic field along z.

Tests/UnitTests/Core/Digitization/PlanarModuleStepperTests.cpp

@@ -46,8 +46,7 @@ auto cSegmentation =
 DigitizationModule pdModule(cSegmentation, hThickness, 1, lAngle, 0., true);
 // (2) negative readout
 DigitizationModule ndModule(cSegmentation, hThickness, -1, lAngle, 0., true);
-std::vector<DigitizationModule> testModules = {std::move(pdModule),
-                                               std::move(ndModule)};
+std::vector<DigitizationModule> testModules = {pdModule, ndModule};
 
 /// The Planar module stepper
 PlanarModuleStepper pmStepper;

Tests/UnitTests/Core/Material/BinnedSurfaceMaterialTests.cpp

@@ -33,9 +33,9 @@ BOOST_AUTO_TEST_CASE(BinnedSurfaceMaterial_construction_test) {
   MaterialSlab a12(Material::fromMolarDensity(6., 7., 8., 9., 10.), 11.);
 
   // Prepare the matrix
-  std::vector<MaterialSlab> l0 = {std::move(a00), std::move(a10)};
-  std::vector<MaterialSlab> l1 = {std::move(a01), std::move(a11)};
-  std::vector<MaterialSlab> l2 = {std::move(a02), std::move(a12)};
+  std::vector<MaterialSlab> l0 = {a00, a10};
+  std::vector<MaterialSlab> l1 = {a01, a11};
+  std::vector<MaterialSlab> l2 = {a02, a12};
 
   // Build the matrix
   std::vector<std::vector<MaterialSlab>> m = {std::move(l0), std::move(l1),

Tests/UnitTests/Core/Material/SurfaceMaterialMapperTests.cpp

@@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE(SurfaceMaterialMapper_tests) {
   Navigator navigator({tGeometry});
   StraightLineStepper stepper;
   SurfaceMaterialMapper::StraightLinePropagator propagator(
-      std::move(stepper), std::move(navigator));
+      stepper, std::move(navigator));
 
   /// The config object
   SurfaceMaterialMapper::Config smmConfig;

Tests/UnitTests/Core/Material/VolumeMaterialMapperTests.cpp

@@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(SurfaceMaterialMapper_tests) {
   /// We need a Navigator, Stepper to build a Propagator
   Navigator navigator({tGeometry});
   StraightLineStepper stepper;
-  VolumeMaterialMapper::StraightLinePropagator propagator(std::move(stepper),
+  VolumeMaterialMapper::StraightLinePropagator propagator(stepper,
                                                           std::move(navigator));
 
   /// The config object

Tests/UnitTests/Core/Propagator/DirectNavigatorTests.cpp

@@ -54,7 +54,7 @@ Stepper estepper(bField);
 Stepper dstepper(bField);
 
 ReferencePropagator rpropagator(std::move(estepper), std::move(navigator));
-DirectPropagator dpropagator(std::move(dstepper), std::move(dnavigator));
+DirectPropagator dpropagator(std::move(dstepper), dnavigator);
 
 const int ntests = 1000;
 const int skip = 0;