Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent GSL_SUPPRESS arguments from being modified by clang-format #17242

Merged
merged 5 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ BasedOnStyle: Google
ColumnLimit: 0
SortIncludes: false
DerivePointerAlignment: false
# Avoid adding spaces between tokens in GSL_SUPPRESS arguments.
# E.g., don't change "GSL_SUPPRESS(r.11)" to "GSL_SUPPRESS(r .11)".
WhitespaceSensitiveMacros: ["GSL_SUPPRESS"]

# if you want to customize when working locally see https://clang.llvm.org/docs/ClangFormatStyleOptions.html for options.
# See ReformatSource.ps1 for a script to update all source according to the current options in this file.
Expand Down
2 changes: 1 addition & 1 deletion include/onnxruntime/core/common/logging/capture.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Capture {

char SeverityPrefix() const noexcept {
// Carefully setup so severity_ is a valid index
GSL_SUPPRESS(bounds .2) {
GSL_SUPPRESS(bounds.2) {
return logging::SEVERITY_PREFIX[static_cast<int>(severity_)];
}
}
Expand Down
18 changes: 5 additions & 13 deletions include/onnxruntime/core/common/narrow.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,13 @@ namespace detail {

// narrow() : a checked version of narrow_cast() that terminates if the cast changed the value
template <class T, class U, typename std::enable_if<std::is_arithmetic<T>::value>::type* = nullptr>
// clang-format off
GSL_SUPPRESS(type.1) // NO-FORMAT: attribute
// clang-format on
constexpr T narrow(U u) noexcept {
GSL_SUPPRESS(type.1) constexpr T narrow(U u) noexcept {
constexpr const bool is_different_signedness =
(std::is_signed<T>::value != std::is_signed<U>::value);

// clang-format off
GSL_SUPPRESS(es.103) // NO-FORMAT: attribute // don't overflow
GSL_SUPPRESS(es.104) // NO-FORMAT: attribute // don't underflow
GSL_SUPPRESS(p.2) // NO-FORMAT: attribute // don't rely on undefined behavior
// clang-format on
GSL_SUPPRESS(es.103) // don't overflow
GSL_SUPPRESS(es.104) // don't underflow
GSL_SUPPRESS(p.2) // don't rely on undefined behavior
const T t = gsl::narrow_cast<T>(u); // While this is technically undefined behavior in some cases (i.e., if the source value is of floating-point type
// and cannot fit into the destination integral type), the resultant behavior is benign on the platforms
// that we target (i.e., no hardware trap representations are hit).
Expand All @@ -59,10 +54,7 @@ GSL_SUPPRESS(p.2) // NO-FORMAT: attribute // don't rely on undefined behavior
}

template <class T, class U, typename std::enable_if<!std::is_arithmetic<T>::value>::type* = nullptr>
// clang-format off
GSL_SUPPRESS(type.1) // NO-FORMAT: attribute
// clang-format on
constexpr T narrow(U u) noexcept {
GSL_SUPPRESS(type.1) constexpr T narrow(U u) noexcept {
const T t = gsl::narrow_cast<T>(u);

if (static_cast<U>(t) != u) {
Expand Down
4 changes: 2 additions & 2 deletions include/onnxruntime/core/common/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ class [[nodiscard]] Status {

Status(StatusCategory category, int code);

GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
Status(const Status& other)
: state_((other.state_ == nullptr) ? nullptr : new State(*other.state_)) {}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
Status& operator=(const Status& other) {
if (state_ != other.state_) {
if (other.state_ == nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,10 @@ class TSharedCubinKernelFactory {
auto const id = hashID(type, sm);
auto const findIter = mKernels.find(id);
if (findIter == mKernels.end()) {
GSL_SUPPRESS(r .11)
auto* newKernel = new TKernelList{pKernelList, nbKernels, type, sm};
auto newKernel = std::make_unique<TKernelList>(pKernelList, nbKernels, type, sm);
newKernel->loadCubinKernels();
mKernels.insert(std::make_pair(id, std::unique_ptr<TKernelList>(newKernel)));
return newKernel;
auto const insert_result = mKernels.insert(std::make_pair(id, std::move(newKernel)));
return insert_result.first->second.get();
}
return findIter->second.get();
}
Expand Down
16 changes: 8 additions & 8 deletions onnxruntime/core/graph/graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ void Node::ForEachDef(std::function<void(const onnxruntime::NodeArg&, bool is_in
// Version ir_version,
// std::unique_ptr<Graph>& new_graph) {
// // create instance. need to call private ctor so can't use make_unique
// GSL_SUPPRESS(r .11)
// GSL_SUPPRESS(r.11)
// new_graph.reset(new Graph(nullptr, &graph_proto, domain_to_version, ir_version));
//
// // as we just loaded from file we want to fully initialize/Resolve, but not let that change
Expand Down Expand Up @@ -1566,7 +1566,7 @@ void Graph::RemoveEdge(NodeIndex src_node_index, NodeIndex dst_node_index, int s
#endif // !defined(ORT_MINIMAL_BUILD) || defined(ORT_EXTENDED_MINIMAL_BUILD)

#if !defined(ORT_MINIMAL_BUILD)
GSL_SUPPRESS(es .84) // ignoring return value from unordered_map::insert causes noisy complaint
GSL_SUPPRESS(es.84) // ignoring return value from unordered_map::insert causes noisy complaint
Status Graph::BuildConnections(std::unordered_set<std::string>& outer_scope_node_args_consumed) {
// recurse into subgraphs first so we can update any nodes in this graph that are used by those subgraphs
if (!resolve_context_.nodes_with_subgraphs.empty()) {
Expand Down Expand Up @@ -1845,7 +1845,7 @@ void Graph::KahnsTopologicalSort(const std::function<void(const Node*)>& enter,
}
}

GSL_SUPPRESS(es .84) // noisy warning about ignoring return value from insert(...)
GSL_SUPPRESS(es.84) // noisy warning about ignoring return value from insert(...)
Status Graph::PerformTopologicalSortAndCheckIsAcyclic() {
nodes_in_topological_order_.clear();
std::unordered_set<NodeIndex> downstream_nodes; // nodes downstream of the node we're currently checking
Expand Down Expand Up @@ -2209,7 +2209,7 @@ Status Graph::UpdateShapeInference(Node& node) {
}

// Implementation of type-inference and type-checking for a single node
GSL_SUPPRESS(f .23) // spurious warning about inferred_type never being checked for null
GSL_SUPPRESS(f.23) // spurious warning about inferred_type never being checked for null
Status Graph::InferAndVerifyTypeMatch(Node& node, const OpSchema& op, const ResolveOptions& options) {
auto& node_name = node.Name();

Expand Down Expand Up @@ -2650,7 +2650,7 @@ Status Graph::VerifyInputAndInitializerNames() {
}

for (auto& initializer_pair : name_to_initial_tensor_) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
inputs_and_initializers.insert(initializer_pair.first);
// Initializers are expected to be included in inputs (according to ONNX spec).
// onnxruntime relaxes this constraint. No duplicate-name check here.
Expand Down Expand Up @@ -3314,7 +3314,7 @@ bool Graph::AddControlEdge(NodeIndex src_node_index, NodeIndex dst_node_index) {
return false;
}

GSL_SUPPRESS(es .84) { // ignoring return from insert()
GSL_SUPPRESS(es.84) { // ignoring return from insert()
nodes_[src_node_index]->MutableRelationships().output_edges.insert(Node::EdgeEnd(*nodes_[dst_node_index]));
nodes_[dst_node_index]->MutableRelationships().input_edges.insert(Node::EdgeEnd(*nodes_[src_node_index]));
nodes_[dst_node_index]->MutableRelationships().control_inputs.insert(nodes_[src_node_index]->Name());
Expand Down Expand Up @@ -3651,7 +3651,7 @@ void Graph::ComputeOverridableInitializers() {

#if !defined(ORT_MINIMAL_BUILD)

GSL_SUPPRESS(es .84) // warning about ignoring return value from insert(...)
GSL_SUPPRESS(es.84) // warning about ignoring return value from insert(...)
Status Graph::SetGraphInputsOutputs() {
// If loaded from a model file, we start from the specified inputs and
// outputs set earlier by InitializeStateFromModelFileGraphProto().
Expand Down Expand Up @@ -3837,7 +3837,7 @@ Status Graph::PopulateNodeArgToProducerConsumerLookupsFromNodes() {
}

// calling private ctor
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
gsl::not_null<Node*> Graph::AllocateNode() {
ORT_ENFORCE(nodes_.size() < static_cast<unsigned int>(std::numeric_limits<int>::max()));
std::unique_ptr<Node> new_node(new Node(nodes_.size(), *this));
Expand Down
18 changes: 9 additions & 9 deletions onnxruntime/core/graph/model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ Model::Model(const std::string& graph_name,
}

// need to call private ctor so can't use make_shared
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
graph_.reset(new Graph(*this, model_proto_.mutable_graph(), *p_domain_to_version, IrVersion(), schema_registry,
logger, options.strict_shape_type_inference));
}
Expand Down Expand Up @@ -238,7 +238,7 @@ Model::Model(ModelProto&& model_proto, const PathString& model_path,
}

// create instance. need to call private ctor so can't use make_unique
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
graph_.reset(new Graph(*this, model_proto_.mutable_graph(), domain_to_version, IrVersion(), schema_registry,
logger, options.strict_shape_type_inference));
}
Expand Down Expand Up @@ -390,7 +390,7 @@ Status Model::Load(const ModelProto& model_proto,
}

// need to call private ctor so can't use make_shared
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)

auto status = Status::OK();
ORT_TRY {
Expand Down Expand Up @@ -430,7 +430,7 @@ Status Model::Load(ModelProto&& model_proto,
}

// need to call private ctor so can't use make_shared
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
auto status = Status::OK();
ORT_TRY {
model = std::make_unique<Model>(std::move(model_proto), model_path, local_registries, logger, options);
Expand Down Expand Up @@ -477,7 +477,7 @@ static Status LoadModelHelper(const T& file_path, Loader loader) {
}

if (!status.IsOK()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
ORT_IGNORE_RETURN_VALUE(Env::Default().FileClose(fd));
return status;
}
Expand Down Expand Up @@ -550,7 +550,7 @@ static Status SaveModel(Model& model, const T& file_path) {
});
}
if (!status.IsOK()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
ORT_IGNORE_RETURN_VALUE(Env::Default().FileClose(fd));
return status;
}
Expand Down Expand Up @@ -583,7 +583,7 @@ static Status SaveModelWithExternalInitializers(Model& model,
});
}
if (!status.IsOK()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
ORT_IGNORE_RETURN_VALUE(Env::Default().FileClose(fd));
return status;
}
Expand All @@ -595,8 +595,8 @@ Status Model::Load(const PathString& file_path,
return LoadModel(file_path, model_proto);
}

GSL_SUPPRESS(r .30) // spurious warnings. p_model is potentially reset in the internal call to Load
GSL_SUPPRESS(r .35)
GSL_SUPPRESS(r.30) // spurious warnings. p_model is potentially reset in the internal call to Load
GSL_SUPPRESS(r.35)
Status Model::Load(const PathString& file_path, std::shared_ptr<Model>& p_model,
const IOnnxRuntimeOpSchemaRegistryList* local_registries,
const logging::Logger& logger, const ModelOptions& options) {
Expand Down
10 changes: 5 additions & 5 deletions onnxruntime/core/graph/schema_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ common::Status OnnxRuntimeOpSchemaRegistry::RegisterOpSchemaInternal(ONNX_NAMESP
<< "than the operator set version " << ver_range_it->second.opset_version << std::endl;
return common::Status(common::ONNXRUNTIME, common::INVALID_ARGUMENT, ostream.str());
}
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
map_[op_name][op_domain].emplace(std::make_pair(ver, op_schema));
return common::Status::OK();
}
Expand Down Expand Up @@ -172,7 +172,7 @@ void SchemaRegistryManager::GetDomainToVersionMapForRegistries(DomainToVersionMa
// If the map doesn't yet contain this domain, insert it with this registry's value.
// Otherwise, merge the existing range in the map.
if (iter == domain_version_map.end()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
domain_version_map.insert(local_domain);
} else {
iter->second = std::max(iter->second, local_domain.second);
Expand All @@ -194,7 +194,7 @@ DomainToVersionMap SchemaRegistryManager::GetLastReleasedOpsetVersions(bool is_o
continue;
auto it = domain_version_map.find(domain.first);
if (it == domain_version_map.end()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
domain_version_map.insert(std::make_pair(domain.first, domain.second));
} else {
it->second = std::max(it->second, domain.second);
Expand All @@ -217,7 +217,7 @@ DomainToVersionMap SchemaRegistryManager::GetLatestOpsetVersions(bool is_onnx_on
continue;
auto it = domain_version_map.find(domain.first);
if (it == domain_version_map.end()) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
domain_version_map.insert(std::make_pair(domain.first, domain.second.second));
} else {
it->second = std::max(it->second, domain.second.second);
Expand Down Expand Up @@ -271,7 +271,7 @@ void SchemaRegistryManager::GetSchemaAndHistory(
}

if (new_version < version) {
GSL_SUPPRESS(es .84)
GSL_SUPPRESS(es.84)
unchecked_registry_indices.insert(unchecked_registry_indices.end(),
checked_registry_indices.begin(),
checked_registry_indices.end());
Expand Down
4 changes: 2 additions & 2 deletions onnxruntime/core/platform/posix/logging/syslog_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ void SysLogSink::SendImpl(const Timestamp& timestamp, const std::string& logger_
msg << timestamp << " [" << message.SeverityPrefix() << ":" << message.Category() << ":" << logger_id << ", "
<< message.Location().ToString() << "] " << message.Message();

GSL_SUPPRESS(bounds .2) {
GSL_SUPPRESS(bounds.2) {
syslog(SYSLOG_LEVEL[static_cast<int>(message.Severity())] - '0', "%s", msg.str().c_str());
}
}

} // namespace logging
} // namespace onnxruntime
} // namespace onnxruntime
2 changes: 1 addition & 1 deletion onnxruntime/core/platform/windows/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ Status WindowsEnv::MapFileIntoMemory(_In_z_ const ORTCHAR_T* file_path,
0,
static_cast<DWORD>(mapped_offset),
mapped_length);
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
mapped_memory =
MappedMemoryPtr{reinterpret_cast<char*>(mapped_base) + offset_to_page,
OrtCallbackInvoker{OrtCallback{UnmapFile, new UnmapFileParam{mapped_base, mapped_length}}}};
Expand Down
10 changes: 5 additions & 5 deletions onnxruntime/core/providers/cpu/activation/activations.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ struct Softplus : public ElementWiseRangedTransform<T> {
Status Init(const onnxruntime::NodeAttributes&) {
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<T>* Copy() const {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type;
Expand All @@ -107,7 +107,7 @@ struct Relu : public ElementWiseRangedTransform<T> {
Status Init(const onnxruntime::NodeAttributes&) {
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<T>* Copy() const { // replace it with a macro. why this?
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type; // redundant?
Expand All @@ -130,7 +130,7 @@ struct Sigmoid : public ElementWiseRangedTransform<T> {
Status Init(const onnxruntime::NodeAttributes&) {
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<T>* Copy() const {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type;
Expand All @@ -156,7 +156,7 @@ struct Softsign : public ElementWiseRangedTransform<T> {
Status Init(const onnxruntime::NodeAttributes&) {
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<T>* Copy() const {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type;
Expand All @@ -179,7 +179,7 @@ struct Tanh : public ElementWiseRangedTransform<T> {
Status Init(const onnxruntime::NodeAttributes&) {
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<T>* Copy() const {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ ElementWiseRangedTransform<T>::~ElementWiseRangedTransform() {
Status Init(const onnxruntime::NodeAttributes& attributes) { \
return (GetFloatParam(#X, attributes, X)); \
} \
GSL_SUPPRESS(r .11) \
GSL_SUPPRESS(r.11) \
ElementWiseRangedTransform<T>* Copy() const final { \
using T1 = typename std::remove_pointer<decltype(this)>::type; \
using T2 = typename std::remove_const<T1>::type; \
Expand All @@ -71,7 +71,7 @@ ElementWiseRangedTransform<T>::~ElementWiseRangedTransform() {
ORT_RETURN_IF_ERROR(GetFloatParam(#Y, attributes, Y)); \
return Status::OK(); \
} \
GSL_SUPPRESS(r .11) \
GSL_SUPPRESS(r.11) \
ElementWiseRangedTransform<T>* Copy() const final { \
using T1 = typename std::remove_pointer<decltype(this)>::type; \
using T2 = typename std::remove_const<T1>::type; \
Expand Down
4 changes: 2 additions & 2 deletions onnxruntime/core/providers/cpu/fp16/fp16_activations.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct Relu<MLFloat16> : public ElementWiseRangedTransform<MLFloat16> {
Activation.ActivationKind = MlasReluActivation;
return Status::OK();
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<MLFloat16>* Copy() const final {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type; // redundant?
Expand Down Expand Up @@ -48,7 +48,7 @@ struct LeakyRelu<MLFloat16> : public ElementWiseRangedTransform<MLFloat16> {
Activation.ActivationKind = MlasLeakyReluActivation;
return (GetFloatParam("alpha", attributes, Activation.Parameters.LeakyRelu.alpha));
}
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
ElementWiseRangedTransform<MLFloat16>* Copy() const final {
using T1 = typename std::remove_pointer<decltype(this)>::type;
using T2 = typename std::remove_const<T1>::type;
Expand Down
4 changes: 2 additions & 2 deletions onnxruntime/core/session/abi_session_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ onnxruntime::Status OrtSessionOptions::RegisterCustomOpsLibrary(onnxruntime::Pat

ORT_API_STATUS_IMPL(OrtApis::CreateSessionOptions, OrtSessionOptions** out) {
API_IMPL_BEGIN
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
*out = new OrtSessionOptions();
return nullptr;
API_IMPL_END
Expand All @@ -72,7 +72,7 @@ ORT_API(void, OrtApis::ReleaseSessionOptions, _Frees_ptr_opt_ OrtSessionOptions*

ORT_API_STATUS_IMPL(OrtApis::CloneSessionOptions, const OrtSessionOptions* input, OrtSessionOptions** out) {
API_IMPL_BEGIN
GSL_SUPPRESS(r .11)
GSL_SUPPRESS(r.11)
*out = new OrtSessionOptions(*input);
return nullptr;
API_IMPL_END
Expand Down
Loading