Address 'cppcoreguidelines-*' clang-tidy remarks

Author: aobolensk

.clang-tidy

@@ -1,10 +1,15 @@
 Checks: >
   bugprone-*,
+  cppcoreguidelines-*,
   modernize-*,
   misc-*,
   performance-*,
   portability-*,
   readability-*,
+  -cppcoreguidelines-avoid-magic-numbers,
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+  -cppcoreguidelines-narrowing-conversions,
+  -cppcoreguidelines-avoid-non-const-global-variables,
   -google-readability-braces-around-statements,
   -google-readability-namespace-comments,
   -google-runtime-references,

src/Weights_Reader/reader_weights.cpp

@@ -44,12 +44,13 @@ json read_json(const std::string& filename) {
   return result;
 
 #else
-  int fd = open(filename.c_str(), O_RDONLY);
+  int fd = open(filename.c_str(),
+                O_RDONLY);  // NOLINT(cppcoreguidelines-pro-type-vararg)
   if (fd == -1) {
     throw std::runtime_error("Cannot open file: " + filename);
   }
 
-  struct stat sb;
+  struct stat sb {};
   fstat(fd, &sb);
 
   if (sb.st_size == 0) {

src/graph/graph.cpp

@@ -26,6 +26,18 @@
 
 namespace it_lab_ai {
 
+namespace {
+template <typename T>
+std::shared_ptr<Layer> clone_layer_checked(
+    const std::shared_ptr<Layer>& layer) {
+  const auto* casted = dynamic_cast<const T*>(layer.get());
+  if (casted == nullptr) {
+    throw std::invalid_argument("Layer type mismatch while cloning");
+  }
+  return std::make_shared<T>(*casted);
+}
+}  // namespace
+
 void Graph::clone(Graph& result, Tensor& out,
                   const RuntimeOptions& options) const {
   result.arrayE_ = this->arrayE_;
@@ -61,110 +73,70 @@ std::shared_ptr<Layer> layer_based_shared_copy(
     const std::shared_ptr<Layer>& layer, const RuntimeOptions& options) {
   switch (layer->getName()) {
     case it_lab_ai::kInput: {
-      auto* tmp_layer = new InputLayer(*dynamic_cast<InputLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<InputLayer>(layer);
     }
     case it_lab_ai::kPooling: {
       if (options.backend == Backend::kOneDnn) {
-        auto* tmp_layer = new PoolingLayerOneDnn(
-            *dynamic_cast<PoolingLayerOneDnn*>(layer.get()));
-        return std::shared_ptr<Layer>(tmp_layer);
+        return clone_layer_checked<PoolingLayerOneDnn>(layer);
       }
-      auto* tmp_layer =
-          new PoolingLayer(*dynamic_cast<PoolingLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<PoolingLayer>(layer);
     }
     case it_lab_ai::kElementWise: {
       if (options.backend == Backend::kOneDnn) {
-        auto* tmp_layer =
-            new EwLayerOneDnn(*dynamic_cast<EwLayerOneDnn*>(layer.get()));
-        return std::shared_ptr<Layer>(tmp_layer);
+        return clone_layer_checked<EwLayerOneDnn>(layer);
       }
-      auto* tmp_layer = new EWLayer(*dynamic_cast<EWLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<EWLayer>(layer);
     }
     case it_lab_ai::kConvolution: {
       if (options.backend == Backend::kOneDnn) {
-        auto* tmp_layer =
-            new ConvLayerOneDnn(*dynamic_cast<ConvLayerOneDnn*>(layer.get()));
-        return std::shared_ptr<Layer>(tmp_layer);
+        return clone_layer_checked<ConvLayerOneDnn>(layer);
       }
-      auto* tmp_layer = new ConvolutionalLayer(
-          *dynamic_cast<ConvolutionalLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<ConvolutionalLayer>(layer);
     }
     case it_lab_ai::kFullyConnected: {
-      auto* tmp_layer = new FCLayer(*dynamic_cast<FCLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<FCLayer>(layer);
     }
     case it_lab_ai::kFlatten: {
-      auto* tmp_layer =
-          new FlattenLayer(*dynamic_cast<FlattenLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<FlattenLayer>(layer);
     }
     case it_lab_ai::kConcat: {
-      auto* tmp_layer =
-          new ConcatLayer(*dynamic_cast<ConcatLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<ConcatLayer>(layer);
     }
     case it_lab_ai::kDropout: {
-      auto* tmp_layer =
-          new DropOutLayer(*dynamic_cast<DropOutLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<DropOutLayer>(layer);
     }
     case it_lab_ai::kSplit: {
-      auto* tmp_layer = new SplitLayer(*dynamic_cast<SplitLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<SplitLayer>(layer);
     }
     case it_lab_ai::kBinaryOp: {
       if (options.backend == Backend::kOneDnn) {
-        auto* tmp_layer = new BinaryOpLayerOneDnn(
-            *dynamic_cast<BinaryOpLayerOneDnn*>(layer.get()));
-        return std::shared_ptr<Layer>(tmp_layer);
+        return clone_layer_checked<BinaryOpLayerOneDnn>(layer);
       }
-      auto* tmp_layer =
-          new BinaryOpLayer(*dynamic_cast<BinaryOpLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<BinaryOpLayer>(layer);
     }
     case it_lab_ai::kTranspose: {
-      auto* tmp_layer =
-          new TransposeLayer(*dynamic_cast<TransposeLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<TransposeLayer>(layer);
     }
     case it_lab_ai::kMatmul: {
-      auto* tmp_layer =
-          new MatmulLayer(*dynamic_cast<MatmulLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<MatmulLayer>(layer);
     }
     case it_lab_ai::kReshape: {
-      auto* tmp_layer =
-          new ReshapeLayer(*dynamic_cast<ReshapeLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<ReshapeLayer>(layer);
     }
     case it_lab_ai::kSoftmax: {
-      auto* tmp_layer =
-          new SoftmaxLayer(*dynamic_cast<SoftmaxLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<SoftmaxLayer>(layer);
     }
     case it_lab_ai::kReduce: {
       if (options.backend == Backend::kOneDnn) {
-        auto* tmp_layer = new ReduceLayerOneDnn(
-            *dynamic_cast<ReduceLayerOneDnn*>(layer.get()));
-        return std::shared_ptr<Layer>(tmp_layer);
+        return clone_layer_checked<ReduceLayerOneDnn>(layer);
       }
-      auto* tmp_layer =
-          new ReduceLayer(*dynamic_cast<ReduceLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<ReduceLayer>(layer);
     }
     case it_lab_ai::kBatchNormalization: {
-      auto* tmp_layer = new BatchNormalizationLayer(
-          *dynamic_cast<BatchNormalizationLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<BatchNormalizationLayer>(layer);
     }
     case it_lab_ai::kOutput: {
-      auto* tmp_layer =
-          new OutputLayer(*dynamic_cast<OutputLayer*>(layer.get()));
-      return std::shared_ptr<Layer>(tmp_layer);
+      return clone_layer_checked<OutputLayer>(layer);
     }
     default: {
       throw std::invalid_argument("No such layer type");

src/graph_transformations/graph_transformations.cpp

@@ -137,8 +137,6 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
   std::vector<int> leaves;
   std::vector<int> roots_inps_final;
   std::vector<int> leaves_outs_final;
-  size_t amount_connected;
-  size_t amount_connected_s;
   for (int v = 0; v < subgraph_from.getLayersCount(); v++) {
     if (is_root(subgraph_from, v)) {
       roots.push_back(v);
@@ -175,8 +173,9 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
         }
       }
       // recognize transformations we can apply with roots
-      amount_connected = new_graph.getOutputsSize(subs[i][roots[j]]);
-      amount_connected_s = subgraph_from.getOutputsSize(roots[j]);
+      const size_t amount_connected =
+          new_graph.getOutputsSize(subs[i][roots[j]]);
+      const size_t amount_connected_s = subgraph_from.getOutputsSize(roots[j]);
       if (amount_connected == amount_connected_s) {
         continue;
       }
@@ -189,7 +188,7 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
       }
     }
     for (int leaf : leaves) {
-      amount_connected = new_graph.getOutputsSize(subs[i][leaf]);
+      const size_t amount_connected = new_graph.getOutputsSize(subs[i][leaf]);
       for (size_t k = 0; k < amount_connected; k++) {
         int id = new_graph.getOutLayers(subs[i][leaf])[k];
         auto it =
@@ -242,8 +241,6 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
   std::vector<int> leaves_to;
   std::vector<std::vector<int>> roots_inps_final;
   std::vector<std::vector<int>> leaves_outs_final;
-  size_t amount_connected;
-  size_t amount_connected_s;
   for (int v = 0; v < subgraph_from.getLayersCount(); v++) {
     if (is_root(subgraph_from, v)) {
       roots_from.push_back(v);
@@ -296,8 +293,10 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
     for (size_t j = 0; j < roots_from.size(); j++) {
       roots_inps_final[j] = new_graph.getInLayers(subs[i][roots_from[j]]);
       // recognize transformations we can apply with roots
-      amount_connected = new_graph.getOutputsSize(subs[i][roots_from[j]]);
-      amount_connected_s = subgraph_from.getOutputsSize(roots_from[j]);
+      const size_t amount_connected =
+          new_graph.getOutputsSize(subs[i][roots_from[j]]);
+      const size_t amount_connected_s =
+          subgraph_from.getOutputsSize(roots_from[j]);
       if (amount_connected == amount_connected_s) {
         continue;
       }
@@ -310,7 +309,8 @@ void changed_subgraphs(const Graph& graph, const Graph& subgraph_from,
       }
     }
     for (size_t j = 0; j < leaves_from.size(); j++) {
-      amount_connected = new_graph.getOutputsSize(subs[i][leaves_from[j]]);
+      const size_t amount_connected =
+          new_graph.getOutputsSize(subs[i][leaves_from[j]]);
       for (size_t k = 0; k < amount_connected; k++) {
         int id = new_graph.getOutLayers(subs[i][leaves_from[j]])[k];
         leaves_outs_final[j].push_back(id);

src/layers/FCLayer.cpp

@@ -17,20 +17,18 @@ void FCLayer::run(const std::vector<Tensor>& input,
     throw std::invalid_argument("Bias and weights data type aren't same");
   }
 
-  size_t batch_size;
-  size_t output_size = bias_->get_shape()[0];
-  if (input[0].get_shape().dims() == 1) {
-    size_t total_elements = input[0].get_shape()[0];
-    size_t expected_input_size = weights_->get_shape()[0];
-
-    if (total_elements % expected_input_size == 0) {
-      batch_size = total_elements / expected_input_size;
-    } else {
-      batch_size = 1;
+  const size_t batch_size = [&]() -> size_t {
+    if (input[0].get_shape().dims() == 1) {
+      const size_t total_elements = input[0].get_shape()[0];
+      const size_t expected_input_size = weights_->get_shape()[0];
+      if (total_elements % expected_input_size == 0) {
+        return total_elements / expected_input_size;
+      }
+      return 1;
     }
-  } else {
-    batch_size = input[0].get_shape()[0];
-  }
+    return input[0].get_shape()[0];
+  }();
+  size_t output_size = bias_->get_shape()[0];
 
   switch (input[0].get_type()) {
     case Type::kInt: {

src/layers/FlattenLayer.cpp

@@ -4,9 +4,8 @@ namespace it_lab_ai {
 
 std::vector<size_t> reorder(std::vector<size_t> order_vec,
                             std::vector<size_t> order) {
-  size_t min_ind;
-  for (size_t i = 0; i < order.size() - 1; i++) {
-    min_ind = i;
+  for (size_t i = 0; i + 1 < order.size(); i++) {
+    size_t min_ind = i;
     for (size_t j = i + 1; j < order.size(); j++) {
       if (order[j] < order[min_ind]) {
         min_ind = j;

src/layers/Shape.cpp

@@ -7,11 +7,11 @@ size_t Shape::get_index(const std::vector<size_t>& coords) const {
     throw std::invalid_argument("Invalid index vector");
   }
   size_t res = 0;
-  size_t mulbuf;
   for (size_t i = 0; i < coords.size(); i++) {
     // to get to the i line
-    mulbuf = std::accumulate(dims_.begin() + (i + 1), dims_.end(),
-                             static_cast<size_t>(1), std::multiplies<>());
+    const size_t mulbuf =
+        std::accumulate(dims_.cbegin() + (i + 1), dims_.cend(),
+                        static_cast<size_t>(1), std::multiplies<>());
     if (coords[i] >= dims_[i]) {
       throw std::out_of_range("Out of range");
     }

src/layers_oneDNN/BinaryOpLayer.cpp

@@ -30,10 +30,16 @@ void BinaryOpLayerOneDnn::run(const std::vector<Tensor>& input,
     const auto& src1_data = *b.as<float>();
     std::vector<float> dst_data(output_shape_.count());
 
-    dnnl::memory src0_mem(src0_md_, *engine_,
-                          const_cast<float*>(src0_data.data()));
-    dnnl::memory src1_mem(src1_md_, *engine_,
-                          const_cast<float*>(src1_data.data()));
+    dnnl::memory src0_mem(
+        src0_md_, *engine_,
+        const_cast<float*>(
+            src0_data
+                .data()));  // NOLINT(cppcoreguidelines-pro-type-const-cast)
+    dnnl::memory src1_mem(
+        src1_md_, *engine_,
+        const_cast<float*>(
+            src1_data
+                .data()));  // NOLINT(cppcoreguidelines-pro-type-const-cast)
     dnnl::memory dst_mem(dst_md_, *engine_, dst_data.data());
 
     binary_prim_->execute(*stream_, {{DNNL_ARG_SRC_0, src0_mem},
@@ -47,10 +53,16 @@ void BinaryOpLayerOneDnn::run(const std::vector<Tensor>& input,
     const auto& src1_data = *b.as<int>();
     std::vector<int> dst_data(output_shape_.count());
 
-    dnnl::memory src0_mem(src0_md_, *engine_,
-                          const_cast<int*>(src0_data.data()));
-    dnnl::memory src1_mem(src1_md_, *engine_,
-                          const_cast<int*>(src1_data.data()));
+    dnnl::memory src0_mem(
+        src0_md_, *engine_,
+        const_cast<int*>(
+            src0_data
+                .data()));  // NOLINT(cppcoreguidelines-pro-type-const-cast)
+    dnnl::memory src1_mem(
+        src1_md_, *engine_,
+        const_cast<int*>(
+            src1_data
+                .data()));  // NOLINT(cppcoreguidelines-pro-type-const-cast)
     dnnl::memory dst_mem(dst_md_, *engine_, dst_data.data());
 
     binary_prim_->execute(*stream_, {{DNNL_ARG_SRC_0, src0_mem},