Prevent GSL_SUPPRESS arguments from being modified by clang-format (#…

…17242)

Prevent `GSL_SUPPRESS` arguments from being modified by clang-format and update existing usages.

clang-format was changing something like `GSL_SUPPRESS(r.11)` to `GSL_SUPPRESS(r .11)`.

For some compilers (e.g., clang), the `gsl::suppress` attribute takes a quoted string argument. We don't want to insert spaces there.

.clang-format

@@ -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.

include/onnxruntime/core/common/logging/capture.h

@@ -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_)];
     }
   }

include/onnxruntime/core/common/narrow.h

@@ -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).
@@ -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) {

include/onnxruntime/core/common/status.h

@@ -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) {

onnxruntime/contrib_ops/cuda/bert/tensorrt_fused_multihead_attention/flash_attention/sharedCubinLoader.h

@@ -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();
   }

onnxruntime/core/graph/graph.cc

@@ -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
@@ -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()) {
@@ -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
@@ -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();
 
@@ -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.
@@ -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());
@@ -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().
@@ -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));

onnxruntime/core/graph/model.cc

@@ -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));
 }
@@ -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));
 }
@@ -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 {
@@ -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);
@@ -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;
   }
@@ -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;
   }
@@ -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;
   }
@@ -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) {

onnxruntime/core/graph/schema_registry.cc

@@ -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();
 }
@@ -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);
@@ -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);
@@ -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);
@@ -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());

onnxruntime/core/platform/posix/logging/syslog_sink.cc

@@ -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

onnxruntime/core/platform/windows/env.cc

@@ -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}}}};

onnxruntime/core/providers/cpu/activation/activations.h

@@ -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;
@@ -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?
@@ -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;
@@ -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;
@@ -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;

onnxruntime/core/providers/cpu/element_wise_ranged_transform.h

@@ -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;               \
@@ -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;               \

onnxruntime/core/providers/cpu/fp16/fp16_activations.h

@@ -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?
@@ -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;

onnxruntime/core/session/abi_session_options.cc

@@ -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
@@ -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