RSDK-3007 - add clang-tidy performance-unnecessary-value-param (viamr…

…obotics#211)

.clang-tidy

@@ -6,7 +6,6 @@
 # readability-named-parameter: Useful to fix lints about unused parameters
 # readability-implicit-bool-conversion: We have decided that !ptr-type is cleaner than ptr-type==nullptr
 # readability-magic-numbers: This encourages useless variables and extra lint lines
-# performance-unnecessary-value-param: TODO(RSDK-3007) remove this and fix all lints.
 # misc-unused-parameters: TODO(RSDK-3008) remove this and fix all lints.
 # misc-include-cleaner: TODO(RSDK-5479) this is overly finnicky, add IWYU support and fix.
 # readability-function-cognitive-complexity: No, complexity is subjective and sometimes necessary.
@@ -27,7 +26,6 @@ Checks: >
   -readability-named-parameter,
   -readability-implicit-bool-conversion,
   -readability-magic-numbers,
-  -performance-unnecessary-value-param,
   -misc-unused-parameters,
   -misc-include-cleaner,
   -readability-function-cognitive-complexity,

src/viam/examples/modules/complex/base/impl.cpp

@@ -34,7 +34,7 @@ std::string find_motor(ResourceConfig cfg, std::string motor_name) {
     return *motor_string;
 }
 
-void MyBase::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MyBase::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     // Downcast `left` and `right` dependencies to motors.
     auto left = find_motor(cfg, "left");
     auto right = find_motor(cfg, "right");

src/viam/examples/modules/complex/base/impl.hpp

@@ -13,10 +13,10 @@ using namespace viam::sdk;
 // specifies a static `validate` method that checks config validity.
 class MyBase : public Base {
    public:
-    MyBase(Dependencies deps, ResourceConfig cfg) : Base(cfg.name()) {
+    MyBase(const Dependencies& deps, const ResourceConfig& cfg) : Base(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     bool is_moving() override;

src/viam/examples/modules/complex/gizmo/impl.cpp

@@ -31,7 +31,7 @@ std::string find_arg1(ResourceConfig cfg) {
     return *arg1_string;
 }
 
-void MyGizmo::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MyGizmo::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     arg1_ = find_arg1(cfg);
 }
 

src/viam/examples/modules/complex/gizmo/impl.hpp

@@ -14,10 +14,10 @@ using namespace viam::sdk;
 class MyGizmo : public Gizmo {
    public:
     MyGizmo(std::string name, std::string arg1) : Gizmo(std::move(name)), arg1_(std::move(arg1)){};
-    MyGizmo(Dependencies deps, ResourceConfig cfg) : Gizmo(cfg.name()) {
+    MyGizmo(const Dependencies& deps, const ResourceConfig& cfg) : Gizmo(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     bool do_one(std::string arg1) override;

src/viam/examples/modules/complex/summation/impl.cpp

@@ -24,7 +24,7 @@ bool find_subtract(ResourceConfig cfg) {
     return *subtract_bool;
 }
 
-void MySummation::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MySummation::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     subtract_ = find_subtract(cfg);
 }
 

src/viam/examples/modules/complex/summation/impl.hpp

@@ -12,10 +12,10 @@ class MySummation : public Summation {
    public:
     MySummation(std::string name, bool subtract)
         : Summation(std::move(name)), subtract_(subtract){};
-    MySummation(Dependencies deps, ResourceConfig cfg) : Summation(cfg.name()) {
+    MySummation(const Dependencies& deps, const ResourceConfig& cfg) : Summation(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     double sum(std::vector<double> numbers) override;

src/viam/sdk/common/linear_algebra.cpp

@@ -45,15 +45,15 @@ std::array<double, 3>& Vector3::data() {
     return this->data_;
 }
 
-viam::common::v1::Vector3 Vector3::to_proto(const Vector3& vec) {
+viam::common::v1::Vector3 Vector3::to_proto() const {
     viam::common::v1::Vector3 result;
-    result.set_x(vec.x());
-    result.set_y(vec.y());
-    result.set_z(vec.z());
+    result.set_x(x());
+    result.set_y(y());
+    result.set_z(z());
     return result;
 };
 
-Vector3 Vector3::from_proto(viam::common::v1::Vector3 vec) {
+Vector3 Vector3::from_proto(const viam::common::v1::Vector3& vec) {
     return {vec.x(), vec.y(), vec.z()};
 }
 

src/viam/sdk/common/linear_algebra.hpp

@@ -30,8 +30,8 @@ class Vector3 {
 
     const std::array<scalar_type, 3>& data() const;
     std::array<scalar_type, 3>& data();
-    static viam::common::v1::Vector3 to_proto(const Vector3& vec);
-    static Vector3 from_proto(viam::common::v1::Vector3 vec);
+    viam::common::v1::Vector3 to_proto() const;
+    static Vector3 from_proto(const viam::common::v1::Vector3& vec);
 
    private:
     std::array<scalar_type, 3> data_;

src/viam/sdk/common/proto_type.cpp

@@ -20,7 +20,7 @@ using google::protobuf::Struct;
 using google::protobuf::Value;
 
 // NOLINTNEXTLINE(misc-no-recursion)
-Struct map_to_struct(AttributeMap dict) {
+Struct map_to_struct(const AttributeMap& dict) {
     Struct s;
     if (!dict) {
         return s;
@@ -37,7 +37,7 @@ Struct map_to_struct(AttributeMap dict) {
 }
 
 // NOLINTNEXTLINE(misc-no-recursion)
-AttributeMap struct_to_map(Struct struct_) {
+AttributeMap struct_to_map(const Struct& struct_) {
     AttributeMap map =
         std::make_shared<std::unordered_map<std::string, std::shared_ptr<ProtoType>>>();
 

src/viam/sdk/common/proto_type.hpp

@@ -83,8 +83,8 @@ class ProtoType {
         proto_type_;
 };
 
-AttributeMap struct_to_map(google::protobuf::Struct struct_);
-google::protobuf::Struct map_to_struct(AttributeMap dict);
+AttributeMap struct_to_map(const google::protobuf::Struct& struct_);
+google::protobuf::Struct map_to_struct(const AttributeMap& dict);
 
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/common/world_state.hpp

@@ -33,7 +33,7 @@ class WorldState {
 
     WorldState() {}
     WorldState(std::vector<geometries_in_frame> obstacles, std::vector<transform> transforms)
-        : obstacles_(obstacles), transforms_(transforms) {}
+        : obstacles_(std::move(obstacles)), transforms_(std::move(transforms)) {}
 
     friend bool operator==(const WorldState& lhs, const WorldState& rhs);
     friend bool operator==(const geometries_in_frame& lhs, const geometries_in_frame& rhs);

src/viam/sdk/components/base/client.cpp

@@ -52,8 +52,8 @@ void BaseClient::set_power(const Vector3& linear,
     return make_client_helper(this, *stub_, &StubType::SetPower)
         .with(extra,
               [&](auto& request) {
-                  *request.mutable_linear() = Vector3::to_proto(linear);
-                  *request.mutable_angular() = Vector3::to_proto(angular);
+                  *request.mutable_linear() = linear.to_proto();
+                  *request.mutable_angular() = angular.to_proto();
               })
         .invoke();
 }
@@ -64,8 +64,8 @@ void BaseClient::set_velocity(const Vector3& linear,
     return make_client_helper(this, *stub_, &StubType::SetVelocity)
         .with(extra,
               [&](auto& request) {
-                  *request.mutable_linear() = Vector3::to_proto(linear);
-                  *request.mutable_angular() = Vector3::to_proto(angular);
+                  *request.mutable_linear() = linear.to_proto();
+                  *request.mutable_angular() = angular.to_proto();
               })
         .invoke();
 }

src/viam/sdk/components/board/board.cpp

@@ -20,7 +20,7 @@ API API::traits<Board>::api() {
     return {kRDK, kComponent, "board"};
 }
 
-Board::status Board::from_proto(viam::common::v1::BoardStatus proto) {
+Board::status Board::from_proto(const viam::common::v1::BoardStatus& proto) {
     Board::status status;
     for (const auto& analog : proto.analogs()) {
         status.analog_reader_values.emplace(analog.first, analog.second.value());
@@ -31,11 +31,11 @@ Board::status Board::from_proto(viam::common::v1::BoardStatus proto) {
     return status;
 }
 
-Board::analog_value Board::from_proto(viam::common::v1::AnalogStatus proto) {
+Board::analog_value Board::from_proto(const viam::common::v1::AnalogStatus& proto) {
     return proto.value();
 }
 
-Board::digital_value Board::from_proto(viam::common::v1::DigitalInterruptStatus proto) {
+Board::digital_value Board::from_proto(const viam::common::v1::DigitalInterruptStatus& proto) {
     return proto.value();
 }
 
@@ -55,7 +55,7 @@ Board::power_mode Board::from_proto(viam::component::board::v1::PowerMode proto)
     }
 }
 
-viam::common::v1::BoardStatus Board::to_proto(status status) {
+viam::common::v1::BoardStatus Board::to_proto(const status& status) {
     viam::common::v1::BoardStatus proto;
     for (const auto& analog : status.analog_reader_values) {
         proto.mutable_analogs()->insert({analog.first, to_proto(analog.second)});

src/viam/sdk/components/board/board.hpp

@@ -52,19 +52,19 @@ class Board : public Component {
     API api() const override;
 
     /// @brief Creates a `status` struct from its proto representation.
-    static status from_proto(viam::common::v1::BoardStatus proto);
+    static status from_proto(const viam::common::v1::BoardStatus& proto);
 
     /// @brief Creates a `analog_value` struct from its proto representation.
-    static analog_value from_proto(viam::common::v1::AnalogStatus proto);
+    static analog_value from_proto(const viam::common::v1::AnalogStatus& proto);
 
     /// @brief Creates a `digital_value` struct from its proto representation.
-    static digital_value from_proto(viam::common::v1::DigitalInterruptStatus proto);
+    static digital_value from_proto(const viam::common::v1::DigitalInterruptStatus& proto);
 
     /// @brief Creates a `power_mode` enum from its proto representation.
     static power_mode from_proto(viam::component::board::v1::PowerMode proto);
 
     /// @brief Converts a `status` struct to its proto representation.
-    static viam::common::v1::BoardStatus to_proto(status status);
+    static viam::common::v1::BoardStatus to_proto(const status& status);
 
     /// @brief Converts a `analog_value` struct to its proto representation.
     static viam::common::v1::AnalogStatus to_proto(analog_value analog_value);

src/viam/sdk/components/camera/camera.cpp

@@ -50,7 +50,8 @@ std::string Camera::format_to_MIME_string(viam::component::camera::v1::Format fo
     }
 }
 
-::viam::component::camera::v1::Format Camera::MIME_string_to_format(std::string mime_string) {
+::viam::component::camera::v1::Format Camera::MIME_string_to_format(
+    const std::string& mime_string) {
     if (mime_string == "image/vnd.viam.rgba") {
         return viam::component::camera::v1::FORMAT_RAW_RGBA;
     }
@@ -66,7 +67,7 @@ ::viam::component::camera::v1::Format Camera::MIME_string_to_format(std::string
     return viam::component::camera::v1::FORMAT_UNSPECIFIED;
 }
 
-Camera::raw_image Camera::from_proto(viam::component::camera::v1::GetImageResponse proto) {
+Camera::raw_image Camera::from_proto(const viam::component::camera::v1::GetImageResponse& proto) {
     Camera::raw_image raw_image;
     std::string img_string = proto.image();
     const std::vector<unsigned char> bytes(img_string.begin(), img_string.end());
@@ -76,7 +77,8 @@ Camera::raw_image Camera::from_proto(viam::component::camera::v1::GetImageRespon
     return raw_image;
 }
 
-Camera::image_collection Camera::from_proto(viam::component::camera::v1::GetImagesResponse proto) {
+Camera::image_collection Camera::from_proto(
+    const viam::component::camera::v1::GetImagesResponse& proto) {
     Camera::image_collection image_collection;
     std::vector<Camera::raw_image> images;
     for (const auto& img : proto.images()) {
@@ -93,7 +95,8 @@ Camera::image_collection Camera::from_proto(viam::component::camera::v1::GetImag
     return image_collection;
 }
 
-Camera::point_cloud Camera::from_proto(viam::component::camera::v1::GetPointCloudResponse proto) {
+Camera::point_cloud Camera::from_proto(
+    const viam::component::camera::v1::GetPointCloudResponse& proto) {
     Camera::point_cloud point_cloud;
     std::string pc_string = proto.point_cloud();
     const std::vector<unsigned char> bytes(pc_string.begin(), pc_string.end());
@@ -103,7 +106,7 @@ Camera::point_cloud Camera::from_proto(viam::component::camera::v1::GetPointClou
 }
 
 Camera::intrinsic_parameters Camera::from_proto(
-    viam::component::camera::v1::IntrinsicParameters proto) {
+    const viam::component::camera::v1::IntrinsicParameters& proto) {
     Camera::intrinsic_parameters params;
     // NOLINTNEXTLINE(bugprone-narrowing-conversions)
     params.width_px = proto.width_px();
@@ -117,14 +120,15 @@ Camera::intrinsic_parameters Camera::from_proto(
 }
 
 Camera::distortion_parameters Camera::from_proto(
-    viam::component::camera::v1::DistortionParameters proto) {
+    const viam::component::camera::v1::DistortionParameters& proto) {
     Camera::distortion_parameters params;
     params.model = proto.model();
     params.parameters = {proto.parameters().begin(), proto.parameters().end()};
     return params;
 }
 
-Camera::properties Camera::from_proto(viam::component::camera::v1::GetPropertiesResponse proto) {
+Camera::properties Camera::from_proto(
+    const viam::component::camera::v1::GetPropertiesResponse& proto) {
     Camera::distortion_parameters distortion_parameters;
     Camera::intrinsic_parameters intrinsic_parameters;
     Camera::properties properties;
@@ -145,7 +149,7 @@ Camera::properties Camera::from_proto(viam::component::camera::v1::GetProperties
 }
 
 viam::component::camera::v1::IntrinsicParameters Camera::to_proto(
-    Camera::intrinsic_parameters params) {
+    const Camera::intrinsic_parameters& params) {
     viam::component::camera::v1::IntrinsicParameters proto;
     proto.set_width_px(params.width_px);
     proto.set_height_px(params.height_px);
@@ -156,7 +160,7 @@ viam::component::camera::v1::IntrinsicParameters Camera::to_proto(
     return proto;
 }
 viam::component::camera::v1::DistortionParameters Camera::to_proto(
-    Camera::distortion_parameters params) {
+    const Camera::distortion_parameters& params) {
     viam::component::camera::v1::DistortionParameters proto;
     *proto.mutable_model() = params.model;
     *proto.mutable_parameters() = {params.parameters.begin(), params.parameters.end()};

src/viam/sdk/components/camera/camera.hpp

@@ -90,32 +90,34 @@ class Camera : public Component {
     static std::string format_to_MIME_string(viam::component::camera::v1::Format format);
 
     /// @brief convert a MIME type string with a protobuf format enum.
-    static viam::component::camera::v1::Format MIME_string_to_format(std::string mime_string);
+    static viam::component::camera::v1::Format MIME_string_to_format(
+        const std::string& mime_string);
 
     /// @brief Creates a `raw_image` struct from its proto representation.
-    static raw_image from_proto(viam::component::camera::v1::GetImageResponse proto);
+    static raw_image from_proto(const viam::component::camera::v1::GetImageResponse& proto);
 
     /// @brief Creates a `image_collection` struct from its proto representation.
-    static image_collection from_proto(viam::component::camera::v1::GetImagesResponse proto);
+    static image_collection from_proto(const viam::component::camera::v1::GetImagesResponse& proto);
 
     /// @brief Creates a `point_cloud` struct from its proto representation.
-    static point_cloud from_proto(viam::component::camera::v1::GetPointCloudResponse proto);
+    static point_cloud from_proto(const viam::component::camera::v1::GetPointCloudResponse& proto);
 
     /// @brief creates an `intrinsic_parameters` struct from its proto representation.
-    static intrinsic_parameters from_proto(viam::component::camera::v1::IntrinsicParameters proto);
+    static intrinsic_parameters from_proto(
+        const viam::component::camera::v1::IntrinsicParameters& proto);
 
     /// @brief creats a `distortion_parameters` struct from its proto representation.
     static distortion_parameters from_proto(
-        viam::component::camera::v1::DistortionParameters proto);
+        const viam::component::camera::v1::DistortionParameters& proto);
 
     /// @brief creates a `properties` struct from its proto representation.
-    static properties from_proto(viam::component::camera::v1::GetPropertiesResponse proto);
+    static properties from_proto(const viam::component::camera::v1::GetPropertiesResponse& proto);
 
     /// @brief converts a `distortion_parameters` struct to its proto representation.
-    static viam::component::camera::v1::DistortionParameters to_proto(distortion_parameters);
+    static viam::component::camera::v1::DistortionParameters to_proto(const distortion_parameters&);
 
     /// @brief converts an `intrinsic_parameters` struct to its proto representation.
-    static viam::component::camera::v1::IntrinsicParameters to_proto(intrinsic_parameters);
+    static viam::component::camera::v1::IntrinsicParameters to_proto(const intrinsic_parameters&);
 
     /// @brief Send/receive arbitrary commands to the resource.
     /// @param Command the command to execute.

src/viam/sdk/components/encoder/encoder.cpp

@@ -38,15 +38,17 @@ Encoder::position_type Encoder::from_proto(viam::component::encoder::v1::Positio
     }
 }
 
-Encoder::position Encoder::from_proto(viam::component::encoder::v1::GetPositionResponse proto) {
+Encoder::position Encoder::from_proto(
+    const viam::component::encoder::v1::GetPositionResponse& proto) {
     Encoder::position position;
     position.value = proto.value();
 
     position.type = from_proto(proto.position_type());
     return position;
 }
 
-Encoder::properties Encoder::from_proto(viam::component::encoder::v1::GetPropertiesResponse proto) {
+Encoder::properties Encoder::from_proto(
+    const viam::component::encoder::v1::GetPropertiesResponse& proto) {
     Encoder::properties properties;
     properties.ticks_count_supported = proto.ticks_count_supported();
 
@@ -72,15 +74,16 @@ viam::component::encoder::v1::PositionType Encoder::to_proto(position_type posit
     }
 }
 
-viam::component::encoder::v1::GetPositionResponse Encoder::to_proto(position position) {
+viam::component::encoder::v1::GetPositionResponse Encoder::to_proto(const position& position) {
     viam::component::encoder::v1::GetPositionResponse proto;
     proto.set_value(position.value);
 
     proto.set_position_type(to_proto(position.type));
     return proto;
 }
 
-viam::component::encoder::v1::GetPropertiesResponse Encoder::to_proto(properties properties) {
+viam::component::encoder::v1::GetPropertiesResponse Encoder::to_proto(
+    const properties& properties) {
     viam::component::encoder::v1::GetPropertiesResponse proto;
     proto.set_ticks_count_supported(properties.ticks_count_supported);