Repository: onnx/onnx
File: onnx/inliner/inliner.cc (C++)

Code before:
void InlineSelectedLocalFunctions(ModelProto& model, const FunctionIdSet& to_inline) {
  InlinerImpl::InlineSelectedLocalFunctions(model, to_inline);
}

void InlineSelectedFunctions(ModelProto& model, const FunctionIdSet& to_inline) {
  InlineSelectedLocalFunctions(model, to_inline);
}

void InlineSelectedFunctions(
    ModelProto& model,
    const FunctionIdSet& to_inline,
    const ISchemaRegistry* schema_registry) {
  if (schema_registry == nullptr) {
    schema_registry = OpSchemaRegistry::Instance();
  }
  InlinerImpl::InlineSelectedFunctions(model, to_inline, schema_registry);
}

// Implementation of the Renamer class using InliningRenamer for proper graph-value attribute handling
class Renamer::Impl : public InliningRenamer {
 private:
  std::unique_ptr<NameGenerator> generator_;

 public:
  Impl(const std::string& prefix, const GraphProto& graph)
      : InliningRenamer("__" + prefix, *(generator_ = std::make_unique<NameGenerator>(graph))) {}

  Impl(const std::string& prefix, const FunctionProto& function)
      : InliningRenamer("__" + prefix, *(generator_ = std::make_unique<NameGenerator>(function))) {}

  std::string BindToUniqueName(const std::string& original_name) {
    // First create the unique name using the inherited MakeUnique method
    std::string unique_name = MakeUnique(original_name);

    // Then bind the original name to the unique name
    ONNX_ASSERT(!rename_scopes.empty());
    auto& current_scope = rename_scopes.back();
    current_scope[original_name] = unique_name;

    return unique_name;
  }

  void BindName(const std::string& formal_name, const std::string& actual_name) {
    auto& current_scope = rename_scopes.back();
    current_scope[formal_name] = actual_name;
  }

  void RenameNode(NodeProto& node) {
    // Use the InliningRenamer's ProcessNode method which handles graph-value attributes
    ProcessNode(&node);
  }

Review comment (bug):
The test-case is crashing in this line. Can you simplify the implementation by replacing  inheritance of InliningRenamer by delegation? Make the NameGenerator an instance of this class, and add an instance of InliningRenamer (using that NameGenerator). Avoid the unique pointer.

Code after:

    for (auto* function_ptr : non_inlined_functions) {
      inliner.ProcessFunction(*function_ptr);
    }

    // Remove all inlined model-local functions.
    auto* local_functions = model.mutable_functions();
    for (auto it = local_functions->begin(); it != local_functions->end();) {
      if (map.count(GetFunctionImplId(*it)) > 0)
        it = local_functions->erase(it);
      else
        ++it;
    }
  }

  static void InlineSelectedLocalFunctions(ModelProto& model, const FunctionIdSet& to_inline) {
    InlineSelectedFunctions(model, to_inline, nullptr);
  }
};

} // namespace

// Public API implementation:

std::unique_ptr<FunctionIdSet> ONNX_NAMESPACE::inliner::FunctionIdSet::Create(
    FunctionIdVector&& function_ids,
    bool invert) {
  return std::make_unique<VectorSet>(std::move(function_ids), invert);
}

void InlineLocalFunctions(ModelProto& model, bool convert_version) {
  InlinerImpl::InlineLocalFunctions(model, convert_version);
}

void InlineSelectedLocalFunctions(ModelProto& model, const FunctionIdSet& to_inline) {
  InlinerImpl::InlineSelectedLocalFunctions(model, to_inline);
}

void InlineSelectedFunctions(ModelProto& model, const FunctionIdSet& to_inline) {
  InlineSelectedLocalFunctions(model, to_inline);
}

void InlineSelectedFunctions(
    ModelProto& model,
    const FunctionIdSet& to_inline,
    const ISchemaRegistry* schema_registry) {
  if (schema_registry == nullptr) {
    schema_registry = OpSchemaRegistry::Instance();
  }
  InlinerImpl::InlineSelectedFunctions(model, to_inline, schema_registry);
}