clang-tidy: resolve performance-unnecessary-value-param

.clang-tidy

@@ -98,7 +98,7 @@ CheckOptions:
   - key: performance-move-const-arg.CheckTriviallyCopyableMove
     value: '1'
   - key: performance-unnecessary-value-param.AllowedTypes
-    value: ''
+    value: 'data_cell_cursor;handle'
 
   # Modernize settings
   - key: modernize-use-override.AllowOverrideAndFinal

docs/dev/clang-tidy-fixes-2026-04.md

@@ -150,7 +150,8 @@
   - Resolved incidentally by [PR #551](https://github.com/Framework-R-D/phlex/pull/551)
 - [x] [performance-unnecessary-copy-initialization](https://clang.llvm.org/extra/clang-tidy/checks/performance/unnecessary-copy-initialization.html) (1)
   - [PR #590](https://github.com/Framework-R-D/phlex/pull/590)
-- [ ] [performance-unnecessary-value-param](https://clang.llvm.org/extra/clang-tidy/checks/performance/unnecessary-value-param.html) (44)
+- [x] [performance-unnecessary-value-param](https://clang.llvm.org/extra/clang-tidy/checks/performance/unnecessary-value-param.html) (44)
+  - [PR #796](https://github.com/Framework-R-D/phlex/pull/796)
 - [x] [portability-template-virtual-member-function](https://clang.llvm.org/extra/clang-tidy/checks/portability/template-virtual-member-function.html) (2)
   - [PR #591](https://github.com/Framework-R-D/phlex/pull/591)
 - [x] [readability-avoid-const-params-in-decls](https://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-const-params-in-decls.html) (7)

form/storage/storage_associative_write_container.cpp

@@ -2,6 +2,8 @@
 
 #include "storage_associative_write_container.hpp"
 
+#include <utility>
+
 using namespace form::detail::experimental;
 
 Storage_Associative_Write_Container::Storage_Associative_Write_Container(std::string const& name) :
@@ -26,5 +28,5 @@ std::string const& Storage_Associative_Write_Container::col_name() { return m_cN
 void Storage_Associative_Write_Container::setParent(
   std::shared_ptr<IStorage_Write_Container> parent)
 {
-  m_parent = parent;
+  m_parent = std::move(parent);
 }

phlex/app/load_module.cpp

@@ -28,9 +28,9 @@ namespace phlex::detail {
       boost::dll::shared_library lib;
       internal::module_creator_t* fn{};
 
-      void operator()(module_graph_proxy<void_tag> proxy, configuration const& config) const
+      void operator()(module_graph_proxy<void_tag> const& proxy, configuration const& config) const
       {
-        fn(std::move(proxy), config);
+        fn(proxy, config);
       }
     };
 
@@ -48,7 +48,9 @@ namespace phlex::detail {
       boost::dll::shared_library lib;
       internal::driver_shim_t* fn{};
 
-      void operator()(driver_proxy proxy, configuration const& config, driver_bundle* out) const
+      void operator()(driver_proxy const& proxy,
+                      configuration const& config,
+                      driver_bundle* out) const
       {
         fn(proxy, config, out);
       }

phlex/core/framework_graph.cpp

@@ -43,7 +43,7 @@ namespace phlex::detail {
                     tbb::flow::unlimited,
                     [this](ready_flushes_then_emit const& input) -> data_cell_index_ptr {
                       auto&& [ready_flushes, index_to_emit] = input;
-                      return index_router_.route(index_to_emit, std::move(ready_flushes));
+                      return index_router_.route(index_to_emit, ready_flushes);
                     }},
     hierarchy_node_{graph_,
                     tbb::flow::unlimited,
@@ -82,7 +82,7 @@ namespace phlex::detail {
     if (shutdown_on_error_) {
       // When in an error state, we need to sanely pop the layer stack and wait for any tasks to finish.
       auto remaining_flushes = cell_tracker_.report_and_evict_ready_flushes(nullptr);
-      index_router_.drain(std::move(remaining_flushes));
+      index_router_.drain(remaining_flushes);
       graph_.wait_for_all();
     }
   }
@@ -213,13 +213,13 @@ namespace phlex::detail {
     }
 
     // Index-router finalization makes edges between the index-set nodes and the provider nodes.
-    finalize_router(std::move(provider_input_ports), std::move(multilayer_join_index_ports));
+    finalize_router(std::move(provider_input_ports), multilayer_join_index_ports);
   }
 
   // FIXME: Much, if not all, of this logic should be moved to the index_router.
   void framework_graph::finalize_router(
     index_router::provider_input_ports_t provider_input_ports,
-    std::map<std::string, named_index_ports> multilayer_join_index_ports)
+    std::map<std::string, named_index_ports> const& multilayer_join_index_ports)
   {
     std::set<phlex::experimental::identifier> unfold_input_layer_names;
 
@@ -248,7 +248,6 @@ namespace phlex::detail {
                                                    unfold_input_layer_names.end()),
       unfold_output_layer_names);
     index_router_.register_unfold_count_per_input_layer(std::move(unfold_count_per_input_layer));
-    index_router_.finalize(
-      graph_, std::move(provider_input_ports), std::move(multilayer_join_index_ports));
+    index_router_.finalize(graph_, std::move(provider_input_ports), multilayer_join_index_ports);
   }
 }

phlex/core/framework_graph.hpp

@@ -28,6 +28,7 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <string_view>
 #include <tuple>
 #include <utility>
 #include <vector>
@@ -56,7 +57,8 @@ namespace phlex::detail {
       requires requires(std::shared_ptr<Generator> generator, std::vector<source const*> sources) {
         { driver_proxy{sources}.driver(generator) } -> std::same_as<driver_bundle>;
       }
-    void add_driver(std::shared_ptr<Generator> generator)
+    void add_driver(
+      std::shared_ptr<Generator> generator) // NOLINT(performance-unnecessary-value-param)
     {
       add_driver(driver_proxy().driver(std::move(generator)));
     }
@@ -76,7 +78,7 @@ namespace phlex::detail {
       return {config, graph_, nodes_, registration_errors_};
     }
 
-    detail::driver_proxy driver_proxy(std::vector<std::string> strings = {})
+    detail::driver_proxy driver_proxy(std::vector<std::string> const& strings = {})
     {
       return detail::driver_proxy{nodes_.sources_for(strings)};
     }
@@ -87,54 +89,57 @@ namespace phlex::detail {
     //      right?
 
     template <typename... InitArgs>
-    auto fold(std::string name,
+    auto fold(std::string_view name,
               is_fold_like auto f,
               concurrency c = concurrency::serial,
               std::string partition = "job",
               InitArgs&&... init_args)
     {
-      return make_glue().fold(std::move(name),
-                              std::move(f),
-                              c,
-                              std::move(partition),
-                              std::forward<InitArgs>(init_args)...);
+      return make_glue().fold(
+        name, std::move(f), c, std::move(partition), std::forward<InitArgs>(init_args)...);
     }
 
     template <typename T>
-    auto unfold(std::string name,
+    auto unfold(std::string_view name,
                 is_predicate_like auto pred,
                 auto unf,
                 concurrency c,
                 std::string destination_data_layer)
     {
       return make_glue<T, false>().unfold(
-        std::move(name), std::move(pred), std::move(unf), c, std::move(destination_data_layer));
+        name, std::move(pred), std::move(unf), c, std::move(destination_data_layer));
     }
 
-    auto observe(std::string name, is_observer_like auto f, concurrency c = concurrency::serial)
+    auto observe(std::string_view name,
+                 is_observer_like auto f,
+                 concurrency c = concurrency::serial)
     {
-      return make_glue().observe(std::move(name), std::move(f), c);
+      return make_glue().observe(name, std::move(f), c);
     }
 
-    auto predicate(std::string name, is_predicate_like auto f, concurrency c = concurrency::serial)
+    auto predicate(std::string_view name,
+                   is_predicate_like auto f,
+                   concurrency c = concurrency::serial)
     {
-      return make_glue().predicate(std::move(name), std::move(f), c);
+      return make_glue().predicate(name, std::move(f), c);
     }
 
-    auto transform(std::string name, is_transform_like auto f, concurrency c = concurrency::serial)
+    auto transform(std::string_view name,
+                   is_transform_like auto f,
+                   concurrency c = concurrency::serial)
     {
-      return make_glue().transform(std::move(name), std::move(f), c);
+      return make_glue().transform(name, std::move(f), c);
     }
 
-    auto provide(std::string name, auto f, concurrency c = concurrency::serial)
+    auto provide(std::string_view name, auto f, concurrency c = concurrency::serial)
     {
-      return make_glue().provide(std::move(name), std::move(f), c);
+      return make_glue().provide(name, std::move(f), c);
     }
 
     template <std::derived_from<source> Source, typename... Args>
-    void add_source(std::string name, Args&&... args)
+    void add_source(std::string_view name, Args&&... args)
     {
-      return make_glue().template add_source<Source>(std::move(name), std::forward<Args>(args)...);
+      return make_glue().template add_source<Source>(name, std::forward<Args>(args)...);
     }
 
     template <typename T, typename... Args>
@@ -184,8 +189,9 @@ namespace phlex::detail {
     void throw_if_registration_errors() const;
     void make_filter_edges();
     void make_bookkeeping_edges();
-    void finalize_router(index_router::provider_input_ports_t provider_input_ports,
-                         std::map<std::string, named_index_ports> multilayer_join_index_ports);
+    void finalize_router(
+      index_router::provider_input_ports_t provider_input_ports,
+      std::map<std::string, named_index_ports> const& multilayer_join_index_ports);
 
     enum class driver_mode { default_driver, deferred_driver };
     explicit framework_graph(driver_mode mode, int max_parallelism);

phlex/core/glue.cpp

@@ -3,9 +3,10 @@
 
 #include <stdexcept>
 #include <string>
+#include <string_view>
 
 namespace phlex::detail::internal {
-  void verify_name(std::string const& name, configuration const* config)
+  void verify_name(std::string_view name, configuration const* config)
   {
     if (not name.empty()) {
       return;

phlex/core/glue.hpp

@@ -15,6 +15,7 @@
 #include <cassert>
 #include <memory>
 #include <string>
+#include <string_view>
 #include <tuple>
 #include <utility>
 
@@ -25,7 +26,7 @@ namespace phlex {
 namespace phlex::detail {
   struct node_catalog;
   namespace internal {
-    PHLEX_CORE_EXPORT void verify_name(std::string const& name, configuration const* config);
+    PHLEX_CORE_EXPORT void verify_name(std::string_view name, configuration const* config);
   }
 
   // ==============================================================================
@@ -52,14 +53,19 @@ namespace phlex::detail {
     {
     }
 
+    // 'f' is a by-value sink: it is moved into algorithm_bits.  The clang-tidy
+    // warning to take it by const reference is a false positive.
     template <typename... InitArgs>
-    auto fold(
-      std::string name, auto f, concurrency c, std::string partition, InitArgs&&... init_args)
+    auto fold(std::string_view name,
+              auto f, // NOLINT(performance-unnecessary-value-param)
+              concurrency c,
+              std::string partition,
+              InitArgs&&... init_args)
     {
       internal::verify_name(name, config_);
       return fold_api{config_,
-                      std::move(name),
-                      algorithm_bits(bound_obj_, std::move(f)),
+                      name,
+                      algorithm_bits(std::move(bound_obj_), std::move(f)),
                       c,
                       graph_,
                       nodes_,
@@ -68,59 +74,75 @@ namespace phlex::detail {
                       std::forward<InitArgs>(init_args)...};
     }
 
+    // 'f' is a by-value sink: it is moved into algorithm_bits.  The clang-tidy
+    // warning to take it by const reference is a false positive.
     template <typename FT>
-    auto observe(std::string name, FT f, concurrency c)
+    auto observe(std::string_view name,
+                 FT f, // NOLINT(performance-unnecessary-value-param)
+                 concurrency c)
     {
       internal::verify_name(name, config_);
       return make_registration<observer_node>(config_,
-                                              std::move(name),
-                                              algorithm_bits{bound_obj_, std::move(f)},
+                                              name,
+                                              algorithm_bits{std::move(bound_obj_), std::move(f)},
                                               c,
                                               graph_,
                                               nodes_,
                                               errors_);
     }
 
+    // 'f' is a by-value sink: it is moved into algorithm_bits.  The clang-tidy
+    // warning to take it by const reference is a false positive.
     template <typename FT>
-    auto provide(std::string name, FT f, concurrency c)
+    auto provide(std::string_view name,
+                 FT f, // NOLINT(performance-unnecessary-value-param)
+                 concurrency c)
     {
       internal::verify_name(name, config_);
       return provider_api{config_,
-                          std::move(name),
-                          algorithm_bits{bound_obj_, std::move(f)},
+                          name,
+                          algorithm_bits{std::move(bound_obj_), std::move(f)},
                           c,
                           graph_,
                           nodes_,
                           errors_};
     }
 
+    // 'f' is a by-value sink: it is moved into algorithm_bits.  The clang-tidy
+    // warning to take it by const reference is a false positive.
     template <typename FT>
-    auto transform(std::string name, FT f, concurrency c)
+    auto transform(std::string_view name,
+                   FT f, // NOLINT(performance-unnecessary-value-param)
+                   concurrency c)
     {
       internal::verify_name(name, config_);
       return make_registration<transform_node>(config_,
-                                               std::move(name),
-                                               algorithm_bits{bound_obj_, std::move(f)},
+                                               name,
+                                               algorithm_bits{std::move(bound_obj_), std::move(f)},
                                                c,
                                                graph_,
                                                nodes_,
                                                errors_);
     }
 
+    // 'f' is a by-value sink: it is moved into algorithm_bits.  The clang-tidy
+    // warning to take it by const reference is a false positive.
     template <typename FT>
-    auto predicate(std::string name, FT f, concurrency c)
+    auto predicate(std::string_view name,
+                   FT f, // NOLINT(performance-unnecessary-value-param)
+                   concurrency c)
     {
       internal::verify_name(name, config_);
       return make_registration<predicate_node>(config_,
-                                               std::move(name),
-                                               algorithm_bits{bound_obj_, std::move(f)},
+                                               name,
+                                               algorithm_bits{std::move(bound_obj_), std::move(f)},
                                                c,
                                                graph_,
                                                nodes_,
                                                errors_);
     }
 
-    auto unfold(std::string name,
+    auto unfold(std::string_view name,
                 auto predicate,
                 auto unfold,
                 concurrency c,
@@ -130,7 +152,7 @@ namespace phlex::detail {
       internal::verify_name(name, config_);
       return unfold_api<T, decltype(predicate), decltype(unfold)>{
         config_,
-        std::move(name),
+        name,
         std::move(predicate),
         std::move(unfold),
         c,
@@ -140,23 +162,23 @@ namespace phlex::detail {
         std::move(destination_data_layer)};
     }
 
-    auto output(std::string name, is_output_like auto f, concurrency c = concurrency::serial)
+    auto output(std::string_view name, is_output_like auto f, concurrency c = concurrency::serial)
     {
       return output_api{nodes_.registrar_for<declared_output_ptr>(errors_),
                         config_,
-                        std::move(name),
+                        name,
                         graph_,
-                        delegate(bound_obj_, f),
+                        delegate(std::move(bound_obj_), f),
                         c};
     }
 
     template <std::derived_from<source> Source, typename... Args>
-    void add_source(std::string name, Args&&... args)
+    void add_source(std::string_view name, Args&&... args)
     {
-      auto [_, inserted] =
-        nodes_.sources.try_emplace(name, std::make_unique<Source>(std::forward<Args>(args)...));
+      auto [_, inserted] = nodes_.sources.try_emplace(
+        std::string{name}, std::make_unique<Source>(std::forward<Args>(args)...));
       if (not inserted) {
-        internal::add_to_error_messages(errors_, "Source", name); // From registrar.hpp
+        internal::add_to_error_messages(errors_, "Source", std::string{name}); // From registrar.hpp
       }
     }