Fix static analyzer warnings (onnx#6449)

cyyever · web-flow · commit 5f8717c698fa · 2024-10-11T15:29:57.000Z
### Description
More fixes for static analyzer warnings, mostly about adding const to
parameter references/pointers.

### Motivation and Context
For better code.
&lt;!-- - Why is this change required? What problem does it solve? --&gt;
&lt;!-- - If it fixes an open issue, please link to the issue here. --&gt;

---------

Signed-off-by: cyy &lt;cyyever@outlook.com&gt;
diff --git a/onnx/common/array_ref.h b/onnx/common/array_ref.h
@@ -23,9 +23,8 @@
 // removed a bunch of slice variants for simplicity...
 
 #pragma once
-#include <assert.h>
-
 #include <array>
+#include <cassert>
 #include <vector>
 
 namespace ONNX_NAMESPACE {
@@ -43,11 +42,11 @@ namespace ONNX_NAMESPACE {
 template <typename T>
 class ArrayRef {
  public:
-  typedef const T* iterator;
-  typedef const T* const_iterator;
-  typedef size_t size_type;
+  using iterator = const T*;
+  using const_iterator = const T*;
+  using size_type = size_t;
 
-  typedef std::reverse_iterator<iterator> reverse_iterator;
+  using reverse_iterator = std::reverse_iterator<iterator>;
 
  private:
   /// The start of the array, in an external buffer.
@@ -64,6 +63,7 @@ class ArrayRef {
   /*implicit*/ ArrayRef() : Data(nullptr), Length(0) {}
 
   /// Construct an ArrayRef from a single element.
+  /// NOLINTNEXTLINE(google-explicit-constructor)
   /*implicit*/ ArrayRef(const T& OneElt) : Data(&OneElt), Length(1) {}
 
   /// Construct an ArrayRef from a pointer and length.
@@ -74,14 +74,17 @@ class ArrayRef {
 
   /// Construct an ArrayRef from a std::vector.
   template <typename A>
+  /// NOLINTNEXTLINE(google-explicit-constructor)
   /*implicit*/ ArrayRef(const std::vector<T, A>& Vec) : Data(Vec.data()), Length(Vec.size()) {}
 
   /// Construct an ArrayRef from a std::array
   template <size_t N>
+  /// NOLINTNEXTLINE(google-explicit-constructor)
   /*implicit*/ constexpr ArrayRef(const std::array<T, N>& Arr) : Data(Arr.data()), Length(N) {}
 
   /// Construct an ArrayRef from a C array.
   template <size_t N>
+  /// NOLINTNEXTLINE(google-explicit-constructor, *array*)
   /*implicit*/ constexpr ArrayRef(const T (&Arr)[N]) : Data(Arr), Length(N) {}
 
   /// Construct an ArrayRef from a std::initializer_list.
@@ -170,14 +173,14 @@ class ArrayRef {
   /// The declaration here is extra complicated so that "arrayRef = {}"
   /// continues to select the move assignment operator.
   template <typename U>
-  typename std::enable_if<std::is_same<U, T>::value, ArrayRef<T>>::type& operator=(U&& Temporary) = delete;
+  std::enable_if_t<std::is_same_v<U, T>, ArrayRef<T>>& operator=(U&& Temporary) = delete;
 
   /// Disallow accidental assignment from a temporary.
   ///
   /// The declaration here is extra complicated so that "arrayRef = {}"
   /// continues to select the move assignment operator.
   template <typename U>
-  typename std::enable_if<std::is_same<U, T>::value, ArrayRef<T>>::type& operator=(std::initializer_list<U>) = delete;
+  std::enable_if_t<std::is_same_v<U, T>, ArrayRef<T>>& operator=(std::initializer_list<U>) = delete;
 
   /// @}
   /// @name Expensive Operations
@@ -189,6 +192,7 @@ class ArrayRef {
   /// @}
   /// @name Conversion operators
   /// @{
+  /// NOLINTNEXTLINE(google-explicit-constructor)
   operator std::vector<T>() const {
     return std::vector<T>(Data, Data + Length);
   }
diff --git a/onnx/common/ir.h b/onnx/common/ir.h
@@ -387,7 +387,7 @@ struct Value final {
   //          %5 = h(%6, %6)
   void replaceAllUsesWith(Value* newValue);
 
-  Value* copyMetadata(Value* from) {
+  Value* copyMetadata(const Value* from) {
     setElemType(from->elemType());
     setSizes(from->sizes());
     if (from->has_unique_name()) {
@@ -728,7 +728,7 @@ struct Node : public Attributes<Node> {
   }
 
   // Check whether this node is before node n in the graph.
-  bool isBefore(Node* n);
+  bool isBefore(const Node* n);
 
   // iterators of the node list starting at this node
   // useful for resuming a search starting at this node
@@ -1354,7 +1354,7 @@ inline void Node::eraseOutput(size_t i) {
   }
 }
 
-inline bool Node::isBefore(Node* n) {
+inline bool Node::isBefore(const Node* n) {
   if (n == nullptr || this == n) {
     // Bail out early.
     return false;
diff --git a/onnx/common/ir_pb_converter.cc b/onnx/common/ir_pb_converter.cc
@@ -193,7 +193,7 @@ static void convertAttribute(const ONNX_NAMESPACE::AttributeProto& ap, Node* n,
   }
 }
 
-static void convertAttributes(ONNX_NAMESPACE::NodeProto& np, Node* n, const int ir_version = IR_VERSION) {
+static void convertAttributes(const ONNX_NAMESPACE::NodeProto& np, Node* n, const int ir_version = IR_VERSION) {
   for (int i = 0; i < np.attribute_size(); i++) {
     convertAttribute(np.attribute(i), n, ir_version);
   }
@@ -217,7 +217,7 @@ static std::vector<Dimension> tensorShapeProtoToDimensions(const ONNX_NAMESPACE:
 }
 
 static void createDummyValue(
-    std::unique_ptr<Graph>& g,
+    const std::unique_ptr<Graph>& g,
     const std::string& name,
     std::unordered_map<std::string, Value*>& value_by_name_of) {
   auto* undef = g->create(kCaptured, 1);
@@ -301,7 +301,7 @@ std::unique_ptr<Graph> graphProtoToGraph(const ONNX_NAMESPACE::GraphProto& gp, b
   }
 
   for (int i = 0; i < gp.node_size(); i++) {
-    auto np = gp.node(i);
+    const auto& np = gp.node(i);
     auto* n = g->create(Symbol(np.op_type()), /* num_outputs = */ np.output_size());
     g->appendNode(n);
     for (int j = 0; j < np.output_size(); j++) {
@@ -406,7 +406,7 @@ std::unique_ptr<Graph> ImportModelProto(const ModelProto& mp) {
 }
 
 // Part 2: convert IR to ONNX Protobuf
-static std::string value_name(Value* n) {
+static std::string value_name(const Value* n) {
   return n->uniqueName();
 }
 
@@ -551,7 +551,7 @@ static void addAttribute(ONNX_NAMESPACE::NodeProto* n_p, Node* n, Symbol name) {
   }
 }
 
-static void encodeTypeProtoTensorType(ONNX_NAMESPACE::TypeProto_Tensor* tensor_type, Value* n) {
+static void encodeTypeProtoTensorType(ONNX_NAMESPACE::TypeProto_Tensor* tensor_type, const Value* n) {
   if (n->elemType() != 0) {
     tensor_type->set_elem_type(n->elemType());
   }
diff --git a/onnx/cpp2py_export.cc b/onnx/cpp2py_export.cc
@@ -7,6 +7,7 @@
 
 #include <climits>
 #include <limits>
+#include <string>
 #include <tuple>
 #include <unordered_map>
 #include <utility>
@@ -47,16 +48,17 @@ static std::string ProtoBytesToText(const py::bytes& bytes) {
 }
 
 template <typename T, typename Ts = std::remove_const_t<T>>
-static std::pair<std::unique_ptr<Ts[]>, std::unordered_map<std::string, T*>> ParseProtoFromBytesMap(
+static std::pair<std::vector<Ts>, std::unordered_map<std::string, T*>> ParseProtoFromBytesMap(
     const std::unordered_map<std::string, py::bytes>& bytesMap) {
-  std::unique_ptr<Ts[]> values(new Ts[bytesMap.size()]);
+  std::vector<Ts> values(bytesMap.size());
   std::unordered_map<std::string, T*> result;
   size_t i = 0;
   for (const auto& kv : bytesMap) {
     ParseProtoFromPyBytes(&values[i], kv.second);
     result[kv.first] = &values[i];
     i++;
   }
+  // C++ guarantees that the pointers remain valid after std::vector<Ts> is moved.
   return std::make_pair(std::move(values), result);
 }
 
diff --git a/onnx/defs/shape_inference.h b/onnx/defs/shape_inference.h
@@ -154,22 +154,22 @@ inline bool getRepeatedAttribute(InferenceContext& ctx, const std::string& attr_
   }
 }
 
-inline int64_t getAttribute(InferenceContext& ctx, const std::string& attributeName, int64_t defaultValue) {
+inline int64_t getAttribute(const InferenceContext& ctx, const std::string& attributeName, int64_t defaultValue) {
   auto attr_proto = ctx.getAttribute(attributeName);
   if ((nullptr != attr_proto) && attr_proto->has_i())
     return attr_proto->i();
   return defaultValue;
 }
 
-inline int64_t getAttribute(DataPropagationContext& ctx, const std::string& attributeName, int64_t defaultValue) {
+inline int64_t getAttribute(const DataPropagationContext& ctx, const std::string& attributeName, int64_t defaultValue) {
   auto attr_proto = ctx.getAttribute(attributeName);
   if ((nullptr != attr_proto) && attr_proto->has_i())
     return attr_proto->i();
   return defaultValue;
 }
 
 inline std::string
-getAttribute(InferenceContext& ctx, const std::string& attributeName, const std::string& defaultValue) {
+getAttribute(const InferenceContext& ctx, const std::string& attributeName, const std::string& defaultValue) {
   auto attr_proto = ctx.getAttribute(attributeName);
   if ((nullptr != attr_proto) && attr_proto->has_s())
     return attr_proto->s();
@@ -352,7 +352,7 @@ inline const TensorShapeProto& getInputShape(const InferenceContext& ctx, size_t
   }
 }
 
-inline const TensorShapeProto* getOptionalInputShape(InferenceContext& ctx, size_t n) {
+inline const TensorShapeProto* getOptionalInputShape(const InferenceContext& ctx, size_t n) {
   const auto* input_type = ctx.getInputType(n);
 
   if (input_type == nullptr) {
@@ -782,7 +782,7 @@ static constexpr T narrow_cast(U&& u) noexcept {
   return static_cast<T>(std::forward<U>(u));
 }
 
-inline void checkInputRank(InferenceContext& ctx, size_t input_index, int expected_rank) {
+inline void checkInputRank(const InferenceContext& ctx, size_t input_index, int expected_rank) {
   // We check the rank only if a rank is known for the input:
   if (hasInputShape(ctx, input_index)) {
     auto rank = getInputShape(ctx, input_index).dim_size();
@@ -842,7 +842,7 @@ inline void unifyDim(const Dim& source_dim, Dim& target_dim) {
   }
 }
 
-inline void unifyInputDim(InferenceContext& ctx, size_t input_index, int dim_index, Dim& dim) {
+inline void unifyInputDim(const InferenceContext& ctx, size_t input_index, int dim_index, Dim& dim) {
   // We unify the dimensions only if it is available for specified input:
   if (hasInputShape(ctx, input_index)) {
     auto& input_shape = getInputShape(ctx, input_index);
diff --git a/onnx/test/cpp/function_verify_test.cc b/onnx/test/cpp/function_verify_test.cc
@@ -14,8 +14,6 @@
 #include "onnx/defs/parser.h"
 #include "onnx/defs/printer.h"
 #include "onnx/defs/schema.h"
-#include "onnx/onnx-operators_pb.h"
-#include "onnx/onnx_pb.h"
 #include "onnx/shape_inference/implementation.h"
 
 namespace ONNX_NAMESPACE {
@@ -92,8 +90,13 @@ static void VerifyTypeConstraint(const OpSchema& function_op, const FunctionProt
         for (auto& actual_type : iter->second) {
           if (allowed_types.find(actual_type) == allowed_types.end()) {
             fail_check(
-                "Input type " + actual_type + " of parameter " + actual_param_name + " of function " +
-                function_op.Name() + " is not allowed by operator " + op_type);
+                "Input type ",
+                actual_type,
+                " of parameter ",
+                actual_param_name + " of function ",
+                function_op.Name(),
+                " is not allowed by operator ",
+                op_type);
           }
         }
       }
@@ -331,7 +334,7 @@ TEST(FunctionVerification, VerifyFunctionOps) {
         VerifyFunction(s, function_body, verified_counter);
       }
     }
-    ONNX_CATCH(ONNX_NAMESPACE::checker::ValidationError e) {
+    ONNX_CATCH(const ONNX_NAMESPACE::checker::ValidationError& e) {
       ONNX_HANDLE_EXCEPTION([&]() { FAIL() << e.what(); });
     }
   }
diff --git a/onnx/test/cpp/shape_inference_test.cc b/onnx/test/cpp/shape_inference_test.cc
@@ -10,15 +10,16 @@
 #include "onnx/defs/parser.h"
 #include "onnx/defs/schema.h"
 #include "onnx/defs/shape_inference.h"
-#include "onnx/onnx_pb.h"
 #include "onnx/shape_inference/implementation.h"
 
 using namespace ONNX_NAMESPACE::shape_inference;
 
 namespace ONNX_NAMESPACE {
 // onnx/defs/controlflow/old.cc
+// NOLINTNEXTLINE(misc-use-internal-linkage)
 void ScanInferenceFunctionOpset8(InferenceContext& ctx);
 // onnx/defs/controlflow/defs.cc
+// NOLINTNEXTLINE(misc-use-internal-linkage)
 void ScanInferenceFunction(InferenceContext& ctx);
 
 namespace Test {
diff --git a/onnx/version_converter/adapters/type_restriction.h b/onnx/version_converter/adapters/type_restriction.h
@@ -26,13 +26,13 @@ class TypeRestriction : public Adapter {
       const std::vector<TensorProto_DataType>& unallowed_types)
       : Adapter(op_name, initial, target), unallowed_types_(unallowed_types) {}
 
-  void adapt_type_restriction(const std::shared_ptr<Graph>&, Node* node) const {
+  void adapt_type_restriction(const std::shared_ptr<Graph>&, const Node* node) const {
     // Since consumed_inputs is optional, no need to add it (as in batchnorm)
     // Iterate over all inputs and outputs
-    for (Value* input : node->inputs()) {
+    for (const Value* input : node->inputs()) {
       isUnallowed(input);
     }
-    for (Value* output : node->outputs()) {
+    for (const Value* output : node->outputs()) {
       isUnallowed(output);
     }
   }
@@ -45,7 +45,7 @@ class TypeRestriction : public Adapter {
  private:
   std::vector<TensorProto_DataType> unallowed_types_;
 
-  void isUnallowed(Value* val) const {
+  void isUnallowed(const Value* val) const {
     ONNX_ASSERTM(
         std::find(std::begin(unallowed_types_), std::end(unallowed_types_), val->elemType()) ==
             std::end(unallowed_types_),

Original file line number	Diff line number	Diff line change
`@@ -193,7 +193,7 @@ static void convertAttribute(const ONNX_NAMESPACE::AttributeProto& ap, Node* n,`
`193`	`193`	`}`
`194`	`194`	`}`
`195`	`195`
`196`		`-static void convertAttributes(ONNX_NAMESPACE::NodeProto& np, Node* n, const int ir_version = IR_VERSION) {`
	`196`	`+static void convertAttributes(const ONNX_NAMESPACE::NodeProto& np, Node* n, const int ir_version = IR_VERSION) {`
`197`	`197`	`for (int i = 0; i < np.attribute_size(); i++) {`
`198`	`198`	`convertAttribute(np.attribute(i), n, ir_version);`
`199`	`199`	`}`
`@@ -217,7 +217,7 @@ static std::vector<Dimension> tensorShapeProtoToDimensions(const ONNX_NAMESPACE:`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	`static void createDummyValue(`
`220`		`- std::unique_ptr<Graph>& g,`
	`220`	`+ const std::unique_ptr<Graph>& g,`
`221`	`221`	`const std::string& name,`
`222`	`222`	`std::unordered_map<std::string, Value*>& value_by_name_of) {`
`223`	`223`	`auto* undef = g->create(kCaptured, 1);`
`@@ -301,7 +301,7 @@ std::unique_ptr<Graph> graphProtoToGraph(const ONNX_NAMESPACE::GraphProto& gp, b`
`301`	`301`	`}`
`302`	`302`
`303`	`303`	`for (int i = 0; i < gp.node_size(); i++) {`
`304`		`- auto np = gp.node(i);`
	`304`	`+ const auto& np = gp.node(i);`
`305`	`305`	`auto* n = g->create(Symbol(np.op_type()), /* num_outputs = */ np.output_size());`
`306`	`306`	`g->appendNode(n);`
`307`	`307`	`for (int j = 0; j < np.output_size(); j++) {`
`@@ -406,7 +406,7 @@ std::unique_ptr<Graph> ImportModelProto(const ModelProto& mp) {`
`406`	`406`	`}`
`407`	`407`
`408`	`408`	`// Part 2: convert IR to ONNX Protobuf`
`409`		`-static std::string value_name(Value* n) {`
	`409`	`+static std::string value_name(const Value* n) {`
`410`	`410`	`return n->uniqueName();`
`411`	`411`	`}`
`412`	`412`
`@@ -551,7 +551,7 @@ static void addAttribute(ONNX_NAMESPACE::NodeProto* n_p, Node* n, Symbol name) {`
`551`	`551`	`}`
`552`	`552`	`}`
`553`	`553`
`554`		`-static void encodeTypeProtoTensorType(ONNX_NAMESPACE::TypeProto_Tensor* tensor_type, Value* n) {`
	`554`	`+static void encodeTypeProtoTensorType(ONNX_NAMESPACE::TypeProto_Tensor* tensor_type, const Value* n) {`
`555`	`555`	`if (n->elemType() != 0) {`
`556`	`556`	`tensor_type->set_elem_type(n->elemType());`
`557`	`557`	`}`
Original file line number	Diff line number	Diff line change
`@@ -154,22 +154,22 @@ inline bool getRepeatedAttribute(InferenceContext& ctx, const std::string& attr_`
`154`	`154`	`}`
`155`	`155`	`}`
`156`	`156`
`157`		`-inline int64_t getAttribute(InferenceContext& ctx, const std::string& attributeName, int64_t defaultValue) {`
	`157`	`+inline int64_t getAttribute(const InferenceContext& ctx, const std::string& attributeName, int64_t defaultValue) {`
`158`	`158`	`auto attr_proto = ctx.getAttribute(attributeName);`
`159`	`159`	`if ((nullptr != attr_proto) && attr_proto->has_i())`
`160`	`160`	`return attr_proto->i();`
`161`	`161`	`return defaultValue;`
`162`	`162`	`}`
`163`	`163`
`164`		`-inline int64_t getAttribute(DataPropagationContext& ctx, const std::string& attributeName, int64_t defaultValue) {`
	`164`	`+inline int64_t getAttribute(const DataPropagationContext& ctx, const std::string& attributeName, int64_t defaultValue) {`
`165`	`165`	`auto attr_proto = ctx.getAttribute(attributeName);`
`166`	`166`	`if ((nullptr != attr_proto) && attr_proto->has_i())`
`167`	`167`	`return attr_proto->i();`
`168`	`168`	`return defaultValue;`
`169`	`169`	`}`
`170`	`170`
`171`	`171`	`inline std::string`
`172`		`-getAttribute(InferenceContext& ctx, const std::string& attributeName, const std::string& defaultValue) {`
	`172`	`+getAttribute(const InferenceContext& ctx, const std::string& attributeName, const std::string& defaultValue) {`
`173`	`173`	`auto attr_proto = ctx.getAttribute(attributeName);`
`174`	`174`	`if ((nullptr != attr_proto) && attr_proto->has_s())`
`175`	`175`	`return attr_proto->s();`
`@@ -352,7 +352,7 @@ inline const TensorShapeProto& getInputShape(const InferenceContext& ctx, size_t`
`352`	`352`	`}`
`353`	`353`	`}`
`354`	`354`
`355`		`-inline const TensorShapeProto* getOptionalInputShape(InferenceContext& ctx, size_t n) {`
	`355`	`+inline const TensorShapeProto* getOptionalInputShape(const InferenceContext& ctx, size_t n) {`
`356`	`356`	`const auto* input_type = ctx.getInputType(n);`
`357`	`357`
`358`	`358`	`if (input_type == nullptr) {`
`@@ -782,7 +782,7 @@ static constexpr T narrow_cast(U&& u) noexcept {`
`782`	`782`	`return static_cast<T>(std::forward<U>(u));`
`783`	`783`	`}`
`784`	`784`
`785`		`-inline void checkInputRank(InferenceContext& ctx, size_t input_index, int expected_rank) {`
	`785`	`+inline void checkInputRank(const InferenceContext& ctx, size_t input_index, int expected_rank) {`
`786`	`786`	`// We check the rank only if a rank is known for the input:`
`787`	`787`	`if (hasInputShape(ctx, input_index)) {`
`788`	`788`	`auto rank = getInputShape(ctx, input_index).dim_size();`
`@@ -842,7 +842,7 @@ inline void unifyDim(const Dim& source_dim, Dim& target_dim) {`
`842`	`842`	`}`
`843`	`843`	`}`
`844`	`844`
`845`		`-inline void unifyInputDim(InferenceContext& ctx, size_t input_index, int dim_index, Dim& dim) {`
	`845`	`+inline void unifyInputDim(const InferenceContext& ctx, size_t input_index, int dim_index, Dim& dim) {`
`846`	`846`	`// We unify the dimensions only if it is available for specified input:`
`847`	`847`	`if (hasInputShape(ctx, input_index)) {`
`848`	`848`	`auto& input_shape = getInputShape(ctx, input_index);`
Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,6 @@`
`14`	`14`	`#include "onnx/defs/parser.h"`
`15`	`15`	`#include "onnx/defs/printer.h"`
`16`	`16`	`#include "onnx/defs/schema.h"`
`17`		`-#include "onnx/onnx-operators_pb.h"`
`18`		`-#include "onnx/onnx_pb.h"`
`19`	`17`	`#include "onnx/shape_inference/implementation.h"`
`20`	`18`
`21`	`19`	`namespace ONNX_NAMESPACE {`
`@@ -92,8 +90,13 @@ static void VerifyTypeConstraint(const OpSchema& function_op, const FunctionProt`
`92`	`90`	`for (auto& actual_type : iter->second) {`
`93`	`91`	`if (allowed_types.find(actual_type) == allowed_types.end()) {`
`94`	`92`	`fail_check(`
`95`		`- "Input type " + actual_type + " of parameter " + actual_param_name + " of function " +`
`96`		`- function_op.Name() + " is not allowed by operator " + op_type);`
	`93`	`+ "Input type ",`
	`94`	`+ actual_type,`
	`95`	`+ " of parameter ",`
	`96`	`+ actual_param_name + " of function ",`
	`97`	`+ function_op.Name(),`
	`98`	`+ " is not allowed by operator ",`
	`99`	`+ op_type);`
`97`	`100`	`}`
`98`	`101`	`}`
`99`	`102`	`}`
`@@ -331,7 +334,7 @@ TEST(FunctionVerification, VerifyFunctionOps) {`
`331`	`334`	`VerifyFunction(s, function_body, verified_counter);`
`332`	`335`	`}`
`333`	`336`	`}`
`334`		`- ONNX_CATCH(ONNX_NAMESPACE::checker::ValidationError e) {`
	`337`	`+ ONNX_CATCH(const ONNX_NAMESPACE::checker::ValidationError& e) {`
`335`	`338`	`ONNX_HANDLE_EXCEPTION([&]() { FAIL() << e.what(); });`
`336`	`339`	`}`
`337`	`340`	`}`