Fix version-converter to generate valid identifiers

onnx/common/ir.h

@@ -39,6 +39,16 @@
 
 namespace ONNX_NAMESPACE {
 
+namespace { // internal/private API
+
+std::string toVarName(size_t i) {
+  std::ostringstream oss;
+  oss << "_v_" << i;
+  return oss.str();
+}
+
+} // namespace
+
 // Graph represents one "function" of computation.
 // It uses a simple ownership model where the graph owns all the nodes inside it.
 // All references inside the graph are raw pointers.
@@ -346,7 +356,7 @@
   std::string uniqueName() const {
     if (has_unique_name())
       return unique_name_;
-    return ONNX_NAMESPACE::to_string(unique());
+    return toVarName(unique());
   }
   Value* setUniqueName(const std::string& name, bool rename_subgraph_captured_nodes = true);
   Value* setStage(size_t s) {
@@ -1034,9 +1044,9 @@
   }
 
   size_t getNextUnique() {
-    std::string next_unique_name = ONNX_NAMESPACE::to_string(++next_unique_);
+    std::string next_unique_name = toVarName(++next_unique_);
     while (!isNameUnique(next_unique_name)) {
-      next_unique_name = ONNX_NAMESPACE::to_string(++next_unique_);
+      next_unique_name = toVarName(++next_unique_);
     }
     return next_unique_;
   }
@@ -1311,7 +1321,7 @@
     newValue->setUniqueName(unique_name);
     // The "unique" semantic of unique_name should be kept or uses()
     // will return an incorrect result when the value is used in subgraph
-    this->setUniqueName(ONNX_NAMESPACE::to_string(graph->getNextUnique()), false);
+    this->setUniqueName(toVarName(graph->getNextUnique()), false);
   }
   newValue->uses_in_current_graph_.reserve(this->uses_in_current_graph_.size());
   for (auto u : uses_in_current_graph_) {

onnx/test/cpp/ir_test.cc

@@ -0,0 +1,60 @@
+// Copyright (c) ONNX Project Contributors
+
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <iostream>
+
+#include "gtest/gtest.h"
+#include "onnx/common/ir.h"
+#include "onnx/common/ir_pb_converter.h"
+#include "onnx/defs/printer.h"
+
+namespace ONNX_NAMESPACE {
+namespace Test {
+
+static bool IsValidIdentifier(const std::string& name) {
+  if (name.empty()) {
+    return false;
+  }
+  if (!isalpha(name[0]) && name[0] != '_') {
+    return false;
+  }
+  for (size_t i = 1; i < name.size(); ++i) {
+    if (!isalnum(name[i]) && name[i] != '_') {
+      return false;
+    }
+  }
+  return true;
+}
+
+TEST(IR, ValidIdentifierTest) {
+  Graph* g = new Graph();
+  g->setName("test");
+  Value* x = g->addInput();
+  x->setUniqueName("x");
+  x->setElemType(ONNX_NAMESPACE::TensorProto_DataType_FLOAT);
+  x->setSizes({Dimension("M"), Dimension("N")});
+  Node* node1 = g->create(kNeg, 1);
+  node1->addInput(x);
+  g->appendNode(node1);
+  Value* temp1 = node1->outputs()[0];
+  Node* node2 = g->create(kNeg, 1);
+  node2->addInput(temp1);
+  g->appendNode(node2);
+  Value* y = node2->outputs()[0];
+  g->registerOutput(y);
+
+  ModelProto model;
+  ExportModelProto(&model, std::shared_ptr<Graph>(g));
+
+  for (auto& node : model.graph().node()) {
+    for (auto& name : node.output()) {
+      EXPECT_TRUE(IsValidIdentifier(name));
+    }
+  }
+}
+
+} // namespace Test
+} // namespace ONNX_NAMESPACE