Skip to content

Commit

Permalink
Fix more static analyzer warnings (onnx#6363)
Browse files Browse the repository at this point in the history
### Description
<!-- - Describe your changes. -->

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Better code.

Signed-off-by: cyy <[email protected]>
  • Loading branch information
cyyever authored Sep 16, 2024
1 parent 060589c commit 84d4f7b
Show file tree
Hide file tree
Showing 21 changed files with 36 additions and 47 deletions.
4 changes: 2 additions & 2 deletions onnx/common/interned_strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ enum BuiltinSymbol {
};

struct Symbol {
Symbol() {}
Symbol() = default;
/*implicit*/ Symbol(BuiltinSymbol value) : value(value) {}
explicit Symbol(const std::string& s);
explicit Symbol(uint32_t value) : value(value) {}
Expand All @@ -210,7 +210,7 @@ struct Symbol {
const char* toString() const;

private:
uint32_t value;
uint32_t value{0};
};

static inline bool operator==(Symbol lhs, Symbol rhs) {
Expand Down
4 changes: 2 additions & 2 deletions onnx/common/ir.h
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,8 @@ class OpSetID final {
// target must be in the form "<domain>&<version>"
static OpSetID fromString(const std::string& target) {
ONNX_TRY {
std::string new_domain = target.substr(0, target.find("$"));
int new_version = ONNX_NAMESPACE::stoi(target.substr(target.find("$") + 1, target.length()).c_str());
std::string new_domain = target.substr(0, target.find('$'));
int new_version = ONNX_NAMESPACE::stoi(target.substr(target.find('$') + 1, target.length()));
return OpSetID(new_domain, new_version);
}
ONNX_CATCH(const std::runtime_error& e) {
Expand Down
1 change: 0 additions & 1 deletion onnx/common/ir_pb_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include "onnx/common/common.h"
#include "onnx/common/ir.h"
#include "onnx/onnx_pb.h"

namespace ONNX_NAMESPACE {

Expand Down
4 changes: 0 additions & 4 deletions onnx/common/model_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

#include "onnx/common/model_helpers.h"

#include "onnx/checker.h"
#include "onnx/defs/schema.h"
#include "onnx/string_utils.h"

namespace ONNX_NAMESPACE {

Common::Status BuildNode(
Expand Down
2 changes: 1 addition & 1 deletion onnx/common/model_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <vector>

#include "onnx/common/status.h"
#include "onnx/onnx-operators_pb.h"
#include "onnx/onnx_pb.h"

namespace ONNX_NAMESPACE {

Expand Down
2 changes: 1 addition & 1 deletion onnx/common/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum StatusCode {

class Status {
public:
Status() noexcept {}
Status() noexcept = default;

Status(StatusCategory category, int code, const std::string& msg);

Expand Down
4 changes: 2 additions & 2 deletions onnx/common/visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct Visitor {
return true;
}

virtual ~Visitor() {}
virtual ~Visitor() = default;
};

// MutableVisitor: A version of Visitor that allows mutation of the visited objects.
Expand Down Expand Up @@ -122,7 +122,7 @@ struct MutableVisitor {
return true;
}

virtual ~MutableVisitor() {}
virtual ~MutableVisitor() = default;
};

} // namespace internal
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/data_propagators.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ inline void GatherOp13DataPropagator(DataPropagationContext& ctx) {
return;
}
const auto input_indices = ctx.getInputData(1);
if (input_data == nullptr || input_indices == nullptr) {
if (input_indices == nullptr) {
return;
}
TensorShapeProto tsp;
Expand Down
13 changes: 3 additions & 10 deletions onnx/defs/function.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,11 @@

#pragma once

#include <mutex>
#include <string>
#include <unordered_map>
#include <utility>
#include <vector>

#include "attr_proto_util.h"
#include "onnx/common/constants.h"
#include "onnx/common/status.h"
#include "onnx/defs/parser.h"
#include "onnx/defs/schema.h"
Expand All @@ -30,16 +27,12 @@ class FunctionBodyHelper {
struct AttributeProtoWrapper {
AttributeProto proto;

AttributeProtoWrapper() {}
AttributeProtoWrapper() = default;

AttributeProtoWrapper(const AttributeProto& attr_prot) {
proto = attr_prot;
}
AttributeProtoWrapper(const AttributeProto& attr_prot) : proto(attr_prot) {}

template <typename T>
AttributeProtoWrapper(const std::string& attr_name, const T& value) {
proto = MakeAttribute(attr_name, value);
}
AttributeProtoWrapper(const std::string& attr_name, const T& value) : proto(MakeAttribute(attr_name, value)) {}
};

struct NodeDef {
Expand Down
6 changes: 3 additions & 3 deletions onnx/defs/printer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class ProtoPrinter {
}

template <typename T>
inline void print(T prim) {
inline void print(const T& prim) {
output_ << prim;
}

Expand Down Expand Up @@ -119,7 +119,7 @@ class ProtoPrinter {
}

template <typename Collection>
inline void printSet(const char* open, const char* separator, const char* close, Collection coll) {
inline void printSet(const char* open, const char* separator, const char* close, const Collection& coll) {
const char* sep = "";
output_ << open;
for (auto& elt : coll) {
Expand All @@ -131,7 +131,7 @@ class ProtoPrinter {
}

template <typename Collection>
inline void printIdSet(const char* open, const char* separator, const char* close, Collection coll) {
inline void printIdSet(const char* open, const char* separator, const char* close, const Collection& coll) {
const char* sep = "";
output_ << open;
for (auto& elt : coll) {
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct FunctionBodyBuildContext {
// getInputType(i) should return null for missing optional inputs, or if
// type-inference could not infer the input-type (erroneous model).
virtual const TypeProto* getInputType(int inputIndex) const = 0;
virtual ~FunctionBodyBuildContext() {}
virtual ~FunctionBodyBuildContext() = default;
};

struct FunctionBodyBuildContextImpl : public FunctionBodyBuildContext {
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/sequence/defs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ bool BuildSequenceMapBodyFunc(
*functionProto.add_input() = MakeString(input_1_name, "_", i);
}

auto schema_outputs = schema.outputs();
auto const& schema_outputs = schema.outputs();
auto output_0_name = schema_outputs[0].GetName();
for (int i = 0; i < noutputs; i++) {
if (!ctx.hasOutput(i))
Expand Down
6 changes: 3 additions & 3 deletions onnx/defs/shape_inference.cc
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ void propagateSequenceElemTypeWithValidation(const TypeProto* input_type, TypePr
fail_type_inference("Input was expected to have sequence type. Got ", input_type->value_case());
}

auto input_seq_type = input_type->sequence_type();
const auto& input_seq_type = input_type->sequence_type();

if (input_seq_type.has_elem_type()) {
propagateElemTypeWithValidation(
Expand All @@ -405,7 +405,7 @@ void propagateOptionalElemTypeWithValidation(const TypeProto* input_type, TypePr
fail_type_inference("Input was expected to have optional type. Got ", input_type->value_case());
}

auto input_opt_type = input_type->optional_type();
const auto& input_opt_type = input_type->optional_type();

if (input_opt_type.has_elem_type()) {
propagateElemTypeWithValidation(
Expand All @@ -424,7 +424,7 @@ void propagateMapElemTypeWithValidation(const TypeProto* input_type, TypeProto*
fail_type_inference("Input was expected to have map type. Got ", input_type->value_case());
}

auto input_map_type = input_type->map_type();
const auto& input_map_type = input_type->map_type();

if (!input_map_type.has_key_type()) {
fail_type_inference("Key type of map input was unknown");
Expand Down
8 changes: 4 additions & 4 deletions onnx/defs/shape_inference.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class InferenceError final : public std::runtime_error {
public:
using std::runtime_error::runtime_error;

InferenceError(const std::string& message) : std::runtime_error(message) {}
explicit InferenceError(const std::string& message) : std::runtime_error(message) {}

const char* what() const noexcept override {
if (!expanded_message_.empty()) {
Expand Down Expand Up @@ -101,7 +101,7 @@ struct InferenceContext {
virtual size_t getNumOutputs() const = 0;
virtual TypeProto* getOutputType(size_t index) = 0;
virtual GraphInferencer* getGraphAttributeInferencer(const std::string& attribute_name) = 0;
virtual ~InferenceContext() {}
virtual ~InferenceContext() = default;
virtual const SparseTensorProto* getInputSparseData(size_t index) const = 0;
// Gets the shape inputs computed by partial data propagation.
virtual const TensorShapeProto* getSymbolicInput(size_t index) const = 0;
Expand All @@ -128,7 +128,7 @@ struct DataPropagationContext {
virtual const TypeProto* getInputType(size_t index) const = 0;
virtual size_t getNumOutputs() const = 0;
virtual const TypeProto* getOutputType(size_t index) const = 0;
virtual ~DataPropagationContext() {}
virtual ~DataPropagationContext() = default;
virtual const TensorShapeProto* getInputData(size_t index) = 0;
virtual void addOutputData(size_t index, TensorShapeProto&& tp) = 0;
};
Expand All @@ -144,7 +144,7 @@ inline void dummyInferenceFunction(InferenceContext&){};
inline void dummyDataPropagationFunction(DataPropagationContext&){};

template <typename T>
inline bool getRepeatedAttribute(InferenceContext& ctx, std::string attr_name, std::vector<T>& values) {
inline bool getRepeatedAttribute(InferenceContext& ctx, const std::string& attr_name, std::vector<T>& values) {
const auto* attr = ctx.getAttribute(attr_name);
if (attr) {
values = RetrieveValues<T>(*attr);
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/tensor_proto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace ONNX_NAMESPACE {
return res; \
} \
/* okay to remove const qualifier as we have already made a copy */ \
char* bytes = const_cast<char*>(raw_data.c_str()); \
char* bytes = raw_data.data(); \
/* onnx is little endian serialized always-tweak byte order if needed */ \
if (!is_processor_little_endian()) { \
const size_t element_size = sizeof(type); \
Expand Down
2 changes: 1 addition & 1 deletion onnx/defs/tensor_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace ONNX_NAMESPACE {
/* make copy as we may have to reverse bytes */ \
std::string raw_data = tensor->raw(); \
/* okay to remove const qualifier as we have already made a copy */ \
char* bytes = const_cast<char*>(raw_data.c_str()); \
char* bytes = raw_data.data(); \
/*onnx is little endian serialized always-tweak byte order if needed*/ \
if (!is_processor_little_endian()) { \
const size_t element_size = sizeof(type); \
Expand Down
2 changes: 1 addition & 1 deletion onnx/inliner/inliner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class InliningRenamer : private MutableVisitor {
}
for (; i < formals.size(); ++i) {
std::string& formal = *formals.Mutable(i);
std::string rename_as = isOutput ? MakeUnique(formal) : std::string("");
std::string rename_as = isOutput ? MakeUnique(formal) : std::string();
current_scope[formal] = rename_as;
if (!rename_as.empty())
formal = rename_as;
Expand Down
2 changes: 1 addition & 1 deletion onnx/test/cpp/function_verify_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ struct FunctionTypeChecker {

for (auto& attribute_vals : *attribute_cases) {
ONNX_TRY {
auto output_types = shape_inference::InferFunctionOutputTypes(function_proto, input_types, attribute_vals);
shape_inference::InferFunctionOutputTypes(function_proto, input_types, attribute_vals);
}
ONNX_CATCH(ONNX_NAMESPACE::InferenceError & e) {
ONNX_HANDLE_EXCEPTION(([&]() { recordError(e.what(), attribute_vals); }));
Expand Down
2 changes: 1 addition & 1 deletion onnx/version_converter/adapters/axes_input_to_attribute.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class AxesInputToAttribute : public Adapter {
ONNX_ASSERTM(
raw_data.size() != 0 && raw_data.size() % 8 == 0,
"Raw Data must be non-empty and size must be a multiple of 8");
int64_t* raw = (int64_t*)const_cast<char*>(raw_data.c_str());
int64_t* raw = reinterpret_cast<int64_t*>(raw_data.data());
node->is_(kaxes, std::vector<int64_t>(raw, raw + node_ptr->t(kvalue).size_from_dim(0)));
} else {
node->is_(kaxes, std::forward<const std::vector<int64_t>>(int64s));
Expand Down
11 changes: 6 additions & 5 deletions onnx/version_converter/adapters/transformers.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@

#include <cinttypes>
#include <string>
#include <utility>
#include <vector>

#include "onnx/common/interned_strings.h"
#include "onnx/version_converter/adapters/adapter.h"

// Node transformers commonly used in version-adapters:

// Capture context by copying values; the graph is unused by these transformers.

#define NODE_TRANSFORMER(node) [=](std::shared_ptr<Graph>, Node * node)
#define NODE_TRANSFORMER(node) [=](const std::shared_ptr<Graph>&, Node* node)

namespace ONNX_NAMESPACE {
namespace version_conversion {
Expand Down Expand Up @@ -63,10 +65,9 @@ inline NodeTransformerFunction SetAttribute(Symbol attr, const std::string& valu
};
}

inline NodeTransformerFunction SetAttribute(Symbol attr, std::vector<int64_t> value) {
inline NodeTransformerFunction SetAttribute(Symbol attr, const std::vector<int64_t>& value) {
return NODE_TRANSFORMER(node) {
std::vector<int64_t> local(value);
node->is_(attr, std::move(local));
node->is_(attr, std::vector<int64_t>(value));
return node;
};
}
Expand Down
2 changes: 1 addition & 1 deletion onnx/version_converter/convert.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ ModelProto ConvertVersion(const ModelProto& mp_in, int target_version) {
// Get initial_opsetid from mp_in
OpSetID initial_struct(0);
for (auto it = mp_in.opset_import().begin(); it != mp_in.opset_import().end(); ++it) {
if (it->domain() == "" || it->domain() == "ai.onnx") {
if (it->domain().empty() || it->domain() == "ai.onnx") {
initial_struct.setVersion(it->version());
break;
}
Expand Down

0 comments on commit 84d4f7b

Please sign in to comment.