Merge branch 'release_ci' of https://github.com/andife/onnx into release_ci

andife · andife · commit 6808589ec708 · 2024-09-26T06:59:20.000+02:00
diff --git a/.clang-tidy b/.clang-tidy
@@ -17,6 +17,7 @@ Checks: >-
   clang-analyzer-alpha.security.*,
   cppcoreguidelines-avoid-goto,
   cppcoreguidelines-interfaces-global-init,
+  cppcoreguidelines-init-variables,
   cppcoreguidelines-no-malloc,
   cppcoreguidelines-prefer-member-initializer,
   cppcoreguidelines-pro-type-member-init,
@@ -33,6 +34,13 @@ Checks: >-
   google-default-arguments,
   google-global-names-in-headers,
   google-explicit-constructor,
+  modernize-make-shared,
+  modernize-make-unique,
+  modernize-pass-by-value,
+  modernize-use-equals-default,
+  modernize-use-equals-delete,
+  modernize-use-nullptr,
+  modernize-use-override,
   modernize-use-emplace
 
 CheckOptions:
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -100,12 +100,10 @@ if(ONNX_DISABLE_EXCEPTIONS)
   endif()
 endif()
 
-# find_package Python has replaced PythonInterp and PythonLibs since cmake 3.12
-# Use the following command in the future; now this is only compatible with the latest pybind11
-# find_package(Python ${PY_VERSION} COMPONENTS Interpreter Development REQUIRED)
-find_package(PythonInterp ${PY_VERSION} REQUIRED)
 if(BUILD_ONNX_PYTHON)
-  find_package(PythonLibs ${PY_VERSION})
+  find_package(Python ${PY_VERSION} COMPONENTS Interpreter Development REQUIRED)
+else()
+  find_package(Python ${PY_VERSION} COMPONENTS Interpreter REQUIRED)
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "AIX")
@@ -360,7 +358,7 @@ function(RELATIVE_PROTOBUF_GENERATE_CPP NAME SRCS HDRS ROOT_DIR DEPEND)
     endif()
 
     add_custom_command(OUTPUT "${GENERATED_PROTO}"
-                       COMMAND "${PYTHON_EXECUTABLE}" "${GEN_PROTO_PY}"
+                       COMMAND Python::Interpreter "${GEN_PROTO_PY}"
                                ARGS ${GEN_PROTO_ARGS}
                        DEPENDS ${INFILE}
                        COMMENT "Running gen_proto.py on ${INFILE}"
@@ -587,7 +585,7 @@ if(BUILD_ONNX_PYTHON)
     target_link_libraries(onnx_cpp2py_export
                           PRIVATE "-Wl,--whole-archive" $<TARGET_FILE:onnx>
                                   "-Wl,--no-whole-archive")
-    # Prevent "undefined symbol: _ZNSt10filesystem7__cxx114path14_M_split_cmptsEv" 
+    # Prevent "undefined symbol: _ZNSt10filesystem7__cxx114path14_M_split_cmptsEv"
     # (std::filesystem::__cxx11::path::_M_split_cmpts()) on gcc 8
     if (CMAKE_CXX_STANDARD EQUAL 17 AND CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
       target_link_libraries(onnx_cpp2py_export PRIVATE "-lstdc++fs")
diff --git a/onnx/checker.h b/onnx/checker.h
@@ -10,11 +10,8 @@
 #include <unordered_set>
 #include <utility>
 
-#include "onnx/defs/function.h"
 #include "onnx/defs/schema.h"
 #include "onnx/onnx-data_pb.h"
-#include "onnx/onnx-operators_pb.h"
-#include "onnx/onnx_pb.h"
 #include "onnx/string_utils.h"
 
 namespace ONNX_NAMESPACE {
@@ -107,6 +104,7 @@ class CheckerContext final {
 class LexicalScopeContext final {
  public:
   LexicalScopeContext() = default;
+  ~LexicalScopeContext() = default;
 
   // Construct an instance with the lexical scope from the parent graph to allow
   // lookup of names from that scope via this_or_ancestor_graph_has.
@@ -119,6 +117,8 @@ class LexicalScopeContext final {
     parent_context_ = &parent_context;
     return *this;
   }
+  LexicalScopeContext(LexicalScopeContext&&) = delete;
+  LexicalScopeContext& operator=(LexicalScopeContext&&) = delete;
 
   void add(const std::string& name) {
     output_names.insert(name);
diff --git a/onnx/common/ir.h b/onnx/common/ir.h
@@ -132,10 +132,10 @@ struct ScalarAttributeValue final : public AttributeValue {
   ValueType& value() {
     return value_;
   }
-  virtual Ptr clone() const override {
+  Ptr clone() const override {
     return std::make_unique<ScalarAttributeValue>(name, value_);
   }
-  virtual AttributeKind kind() const override {
+  AttributeKind kind() const override {
     return Kind;
   }
 
@@ -147,14 +147,14 @@ template <typename T, AttributeKind Kind>
 struct VectorAttributeValue final : public AttributeValue {
   using ConstructorType = const std::vector<T>&&;
   using ValueType = std::vector<T>;
-  VectorAttributeValue(Symbol name, ConstructorType value_) : AttributeValue(name), value_(std::move(value_)) {}
+  VectorAttributeValue(Symbol name, ValueType value_) : AttributeValue(name), value_(std::move(value_)) {}
   ValueType& value() {
     return value_;
   }
-  virtual AttributeKind kind() const override {
+  AttributeKind kind() const override {
     return Kind;
   }
-  virtual std::unique_ptr<AttributeValue> clone() const override {
+  std::unique_ptr<AttributeValue> clone() const override {
     return std::make_unique<VectorAttributeValue>(name, ValueType(value_));
   }
 
@@ -181,7 +181,7 @@ using TypeProtosAttr = VectorAttributeValue<TypeProto, AttributeKind::tps>;
 // we return Derived* pointers because Nodes are normally held as pointers.
 template <typename Derived>
 struct Attributes {
-  Attributes() {}
+  Attributes() = default;
   void copyAttributes(const Attributes& rhs) {
     values_.clear();
     values_.reserve(rhs.values_.size());
@@ -375,7 +375,7 @@ struct Value final {
   Graph* owningGraph();
   const Graph* owningGraph() const;
   // TODO: make this more const correct
-  const use_list uses() const;
+  use_list uses() const;
 
   // Replaces all uses of this node with 'newValue'.
   //
@@ -839,7 +839,7 @@ class OpSetID final {
   // Default Domain Constructor
   explicit OpSetID(const int64_t version) : domain_(""), version_(version) {}
 
-  explicit OpSetID(const std::string& domain, int64_t version) : domain_(domain), version_(version) {}
+  explicit OpSetID(std::string domain, int64_t version) : domain_(std::move(domain)), version_(version) {}
 
   // target must be in the form "<domain>&<version>"
   std::string toString() const {
@@ -1425,7 +1425,7 @@ inline const_graph_node_list_iterator Node::reverseIterator() const {
 // nodes in subgraph are also included.
 // This method is usually used to check whether it is
 // safe to delete a Value.
-inline const use_list Value::uses() const {
+inline use_list Value::uses() const {
   use_list all_uses = uses_in_current_graph_;
   owningGraph()->forEachNode([this, &all_uses](const Node* node) {
     if (node->owningGraph() == this->owningGraph()) {
diff --git a/onnx/common/ir_pb_converter.cc b/onnx/common/ir_pb_converter.cc
@@ -227,7 +227,7 @@ void createDummyValue(
 }
 
 std::unique_ptr<Graph> graphProtoToGraph(const ONNX_NAMESPACE::GraphProto& gp, bool nested, const int ir_version) {
-  std::unique_ptr<Graph> g(new Graph());
+  auto g = std::make_unique<Graph>();
 
   if (gp.has_name()) {
     g->setName(gp.name());
diff --git a/onnx/common/model_helpers.cc b/onnx/common/model_helpers.cc
@@ -14,7 +14,7 @@ Common::Status BuildNode(
     std::vector<std::string> const& inputs,
     std::vector<std::string> const& outputs,
     NodeProto* node) {
-  if (node == NULL) {
+  if (node == nullptr) {
     return Common::Status(Common::CHECKER, Common::INVALID_ARGUMENT, "node_proto should not be nullptr.");
   }
   node->set_name(name);
diff --git a/onnx/common/status.cc b/onnx/common/status.cc
@@ -8,20 +8,22 @@
 
 #include <assert.h>
 
+#include <memory>
+
 #include "onnx/string_utils.h"
 
 namespace ONNX_NAMESPACE {
 namespace Common {
 
 Status::Status(StatusCategory category, int code, const std::string& msg) {
   assert(static_cast<int>(StatusCode::OK) != code);
-  state_.reset(new State(category, code, msg));
+  state_ = std::make_unique<State>(category, code, msg);
 }
 
 Status::Status(StatusCategory category, int code) : Status(category, code, EmptyString()) {}
 
 bool Status::IsOK() const noexcept {
-  return (state_ == NULL);
+  return (state_ == nullptr);
 }
 
 StatusCategory Status::Category() const noexcept {
diff --git a/onnx/common/status.h b/onnx/common/status.h
@@ -44,7 +44,7 @@ class Status {
       if (nullptr == other.state_) {
         state_.reset();
       } else if (state_ != other.state_) {
-        state_.reset(new State(*other.state_));
+        state_ = std::make_unique<State>(*other.state_);
       }
     }
   }
diff --git a/onnx/defs/data_type_utils.cc b/onnx/defs/data_type_utils.cc
@@ -186,8 +186,7 @@ void DataTypeUtils::FromString(const std::string& type_str, TypeProto& type_prot
     s.LStrip(key_size);
     s.LStrip(",");
     StringRange v(s.Data(), s.Size());
-    int32_t key_type;
-    FromDataTypeString(key, key_type);
+    auto key_type = FromDataTypeString(key);
     type_proto.mutable_map_type()->set_key_type(key_type);
     return FromString(std::string(v.Data(), v.Size()), *type_proto.mutable_map_type()->mutable_value_type());
   } else
@@ -211,18 +210,15 @@ void DataTypeUtils::FromString(const std::string& type_str, TypeProto& type_prot
 #endif
       if (s.LStrip("sparse_tensor")) {
     s.ParensWhitespaceStrip();
-    int32_t e;
-    FromDataTypeString(std::string(s.Data(), s.Size()), e);
+    auto e = FromDataTypeString(std::string(s.Data(), s.Size()));
     type_proto.mutable_sparse_tensor_type()->set_elem_type(e);
   } else if (s.LStrip("tensor")) {
     s.ParensWhitespaceStrip();
-    int32_t e;
-    FromDataTypeString(std::string(s.Data(), s.Size()), e);
+    auto e = FromDataTypeString(std::string(s.Data(), s.Size()));
     type_proto.mutable_tensor_type()->set_elem_type(e);
   } else {
     // Scalar
-    int32_t e;
-    FromDataTypeString(std::string(s.Data(), s.Size()), e);
+    auto e = FromDataTypeString(std::string(s.Data(), s.Size()));
     TypeProto::Tensor* t = type_proto.mutable_tensor_type();
     t->set_elem_type(e);
     // Call mutable_shape() to initialize a shape with no dimension.
@@ -236,14 +232,14 @@ bool DataTypeUtils::IsValidDataTypeString(const std::string& type_str) {
   return (allowedSet.find(type_str) != allowedSet.end());
 }
 
-void DataTypeUtils::FromDataTypeString(const std::string& type_str, int32_t& tensor_data_type) {
+int32_t DataTypeUtils::FromDataTypeString(const std::string& type_str) {
   if (!IsValidDataTypeString(type_str)) {
     ONNX_THROW_EX(std::invalid_argument(
         "DataTypeUtils::FromDataTypeString - Received invalid data type string '" + type_str + "'."));
   }
 
   TypesWrapper& t = TypesWrapper::GetTypesWrapper();
-  tensor_data_type = t.TypeStrToTensorDataType()[type_str];
+  return t.TypeStrToTensorDataType()[type_str];
 }
 
 StringRange::StringRange() : data_(""), size_(0), start_(data_), end_(data_) {}
diff --git a/onnx/defs/data_type_utils.h b/onnx/defs/data_type_utils.h
@@ -52,7 +52,7 @@ class DataTypeUtils final {
  private:
   static void FromString(const std::string& type_str, TypeProto& type_proto);
 
-  static void FromDataTypeString(const std::string& type_str, int32_t& tensor_data_type);
+  static int32_t FromDataTypeString(const std::string& type_str);
 
   static std::string ToString(const TypeProto& type_proto, const std::string& left = "", const std::string& right = "");
 
diff --git a/onnx/defs/function.h b/onnx/defs/function.h
@@ -29,7 +29,7 @@ class FunctionBodyHelper {
 
     AttributeProtoWrapper() = default;
 
-    AttributeProtoWrapper(const AttributeProto& attr_prot) : proto(attr_prot) {}
+    AttributeProtoWrapper(AttributeProto attr_prot) : proto(std::move(attr_prot)) {}
 
     template <typename T>
     AttributeProtoWrapper(const std::string& attr_name, const T& value) : proto(MakeAttribute(attr_name, value)) {}
diff --git a/onnx/defs/math/old.cc b/onnx/defs/math/old.cc
@@ -3579,8 +3579,8 @@ ONNX_OPERATOR_SET_SCHEMA(
 
           // Check for compatible matrix multiply dimensions
           {
-            auto dimL = shapeL.dim(shapeL.dim_size() - 1);
-            auto dimR = shapeR.dim(shapeR.dim_size() - 2);
+            auto const& dimL = shapeL.dim(shapeL.dim_size() - 1);
+            auto const& dimR = shapeR.dim(shapeR.dim_size() - 2);
             if (dimL.has_dim_value() && dimR.has_dim_value() && dimL.dim_value() != dimR.dim_value()) {
               fail_shape_inference("Incompatible dimensions for matrix multiplication");
               ;
diff --git a/onnx/defs/nn/defs.cc b/onnx/defs/nn/defs.cc
@@ -1215,7 +1215,7 @@ void convTransposeShapeInference(InferenceContext& ctx) {
       ctx.getInputType(1)->tensor_type().shape().dim(1) * group; // channels should be the second dim of second input
                                                                  // multiply group.
 
-  int size_of_output;
+  int size_of_output = 0;
   if (output_shape_presented) {
     size_of_output = static_cast<int>(output_shape.size());
     for (int i = 0; i < size_of_output; ++i) {
diff --git a/onnx/defs/nn/old.cc b/onnx/defs/nn/old.cc
@@ -251,7 +251,7 @@ void convPoolShapeInference_opset19(
 
     // how many times we can move the kernel from it's initial position, based
     // on the stride
-    int64_t strided_kernel_positions;
+    int64_t strided_kernel_positions = 0;
 
     if (ceil_mode == 1)
       strided_kernel_positions =
@@ -726,7 +726,7 @@ void convTransposeShapeInference_opset11(InferenceContext& ctx) {
       ctx.getInputType(1)->tensor_type().shape().dim(1) * group; // channels should be the second dim of second input
                                                                  // multiply group.
 
-  int size_of_output;
+  int size_of_output = 0;
   if (output_shape_presented) {
     size_of_output = static_cast<int>(output_shape.size());
     for (int i = 0; i < size_of_output; ++i) {
@@ -2013,7 +2013,7 @@ void convPoolShapeInference1(
 
     // how many times we can move the kernel from it's initial position, based
     // on the stride
-    int64_t strided_kernel_positions;
+    int64_t strided_kernel_positions = 0;
 
     if (ceil_mode == 1)
       strided_kernel_positions =
@@ -3004,7 +3004,7 @@ void convTransposeShapeInference1(InferenceContext& ctx) {
       ctx.getInputType(1)->tensor_type().shape().dim(1) * group; // channels should be the second dim of second input
                                                                  // multiply group.
 
-  int size_of_output;
+  int size_of_output = 0;
   if (output_shape_presented) {
     size_of_output = static_cast<int>(output_shape.size());
     for (int i = 0; i < size_of_output; ++i) {
diff --git a/onnx/defs/parser.cc b/onnx/defs/parser.cc
@@ -444,7 +444,7 @@ Status OnnxParser::Parse(TensorProto& tensorProto, const TypeProto& tensorTypePr
   // tensorProto.mutable_int64_data()->Reserve(n);
   // Parse the actual values:
 
-  int64_t intval;
+  int64_t intval = 0;
   uint64_t uintval = 0;
   float floatval = 0.0;
   double dblval = 0.0;
@@ -833,7 +833,7 @@ Status OnnxParser::Parse(OpsetIdList& opsets) {
 Status OnnxParser::Parse(ModelProto& model) {
   model.Clear();
   std::string strval;
-  int64_t intval;
+  int64_t intval = 0;
   if (Matches('<')) {
     do {
       KeyWordMap::KeyWord keyword = KeyWordMap::KeyWord::NONE;
diff --git a/onnx/defs/parser.h b/onnx/defs/parser.h
@@ -9,8 +9,6 @@
 
 #include <ctype.h>
 
-#include <iostream>
-#include <stdexcept>
 #include <string>
 #include <unordered_map>
 
diff --git a/onnx/defs/printer.cc b/onnx/defs/printer.cc
@@ -5,7 +5,6 @@
 #include "onnx/defs/printer.h"
 
 #include <iomanip>
-#include <vector>
 
 #include "onnx/defs/tensor_proto_util.h"
 
diff --git a/onnx/defs/schema.cc b/onnx/defs/schema.cc
diff --git a/onnx/defs/schema.h b/onnx/defs/schema.h
diff --git a/onnx/shape_inference/implementation.h b/onnx/shape_inference/implementation.h
diff --git a/onnx/version_converter/convert.cc b/onnx/version_converter/convert.cc
diff --git a/onnx/version_converter/helper.cc b/onnx/version_converter/helper.cc

Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,7 @@ void createDummyValue(`
`227`	`227`	`}`
`228`	`228`
`229`	`229`	`std::unique_ptr<Graph> graphProtoToGraph(const ONNX_NAMESPACE::GraphProto& gp, bool nested, const int ir_version) {`
`230`		`- std::unique_ptr<Graph> g(new Graph());`
	`230`	`+ auto g = std::make_unique<Graph>();`
`231`	`231`
`232`	`232`	`if (gp.has_name()) {`
`233`	`233`	`g->setName(gp.name());`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ Common::Status BuildNode(`
`14`	`14`	`std::vector<std::string> const& inputs,`
`15`	`15`	`std::vector<std::string> const& outputs,`
`16`	`16`	`NodeProto* node) {`
`17`		`- if (node == NULL) {`
	`17`	`+ if (node == nullptr) {`
`18`	`18`	`return Common::Status(Common::CHECKER, Common::INVALID_ARGUMENT, "node_proto should not be nullptr.");`
`19`	`19`	`}`
`20`	`20`	`node->set_name(name);`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ class Status {`
`44`	`44`	`if (nullptr == other.state_) {`
`45`	`45`	`state_.reset();`
`46`	`46`	`} else if (state_ != other.state_) {`
`47`		`- state_.reset(new State(*other.state_));`
	`47`	`+ state_ = std::make_unique<State>(*other.state_);`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`	`}`