Skip to content

[node] remove incorrect error check - #26467

Merged
Yulong Wang (fs-eire) merged 1 commit into
mainfrom
fs-eire/fix-warning-node-binding
Nov 4, 2025
Merged

[node] remove incorrect error check#26467
Yulong Wang (fs-eire) merged 1 commit into
mainfrom
fs-eire/fix-warning-node-binding

Conversation

@fs-eire

@fs-eire Yulong Wang (fs-eire) commented Nov 1, 2025

Copy link
Copy Markdown
Contributor

Description

remove the code that generates warning.

[6/8] Building CXX object CMakeFiles/o...ing.dir/src/inference_session_wrap.cc.
/s/js/node/src/inference_session_wrap.cc:19:8: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to false [-Wtautological-undefined-compare]
   19 |       &Ort::GetApi() == nullptr, env,
      |        ^~~~~~~~~~~~~    ~~~~~~~
/s/js/node/src/common.h:41:67: note: expanded from macro 'ORT_NAPI_THROW_ERROR_IF'
   41 | #define ORT_NAPI_THROW_ERROR_IF(COND, ENV, ...) ORT_NAPI_THROW_IF(COND, Error, ENV, __VA_ARGS__)
      |                                                                   ^~~~
/s/js/node/src/common.h:38:7: note: expanded from macro 'ORT_NAPI_THROW_IF'
   38 |   if (COND) {                                    \
      |       ^~~~
/s/js/node/../../include/onnxruntime/core/session/onnxruntime_cxx_api.h:189:22: note: 'GetApi' returns a reference
  189 | inline const OrtApi& GetApi() noexcept { return *detail::Global::Api(); }
      |                      ^
1 warning generated.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes a defunct API initialization check from the Node.js binding initialization code. The removed check attempted to verify ONNX Runtime API initialization but was ineffective due to checking the address of a reference rather than a pointer.

  • Removes an ineffective null-check for the ONNX Runtime API
  • Simplifies the initialization flow by removing unreachable error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fs-eire
Yulong Wang (fs-eire) enabled auto-merge (squash) November 1, 2025 17:49
Comment thread js/node/src/inference_session_wrap.cc
@fs-eire
Yulong Wang (fs-eire) merged commit b6122c6 into main Nov 4, 2025
98 checks passed
@fs-eire
Yulong Wang (fs-eire) deleted the fs-eire/fix-warning-node-binding branch November 4, 2025 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants