From 817d70a63b62dcab40c2fd038ca0dc30072cb5be Mon Sep 17 00:00:00 2001 From: Rachel Guo <35738743+YUNQIUGUO@users.noreply.github.com> Date: Mon, 8 May 2023 17:12:10 -0700 Subject: [PATCH] [js/rn] Fix extensions header include issue (#15800) ### Description Identified the cause for a `redefinition compilation error` happened in a react native expo app with ort-extensions enabled when running the ios side. Fix the include path now, so we can remove the temporary forward declaration in OnnxruntimeModule.mm file. ### Motivation and Context Fix implementation detail. --------- Co-authored-by: rachguo --- js/react_native/ios/OnnxruntimeModule.mm | 21 +++++++++++---------- js/react_native/ios/TensorHelper.h | 14 +++++++++++++- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/js/react_native/ios/OnnxruntimeModule.mm b/js/react_native/ios/OnnxruntimeModule.mm index 1b2b52c231..15ede97958 100644 --- a/js/react_native/ios/OnnxruntimeModule.mm +++ b/js/react_native/ios/OnnxruntimeModule.mm @@ -6,18 +6,19 @@ #import #import -#import +// Note: Using below syntax for including ort c api and ort extensions headers to resolve a compiling error happened +// in an expo react native ios app when ort extensions enabled (a redefinition error of multiple object types defined +// within ORT C API header). It's an edge case that compiler allows both ort c api headers to be included when #include +// syntax doesn't match. For the case when extensions not enabled, it still requires a onnxruntime prefix directory for +// searching paths. Also in general, it's a convention to use #include for C/C++ headers rather then #import. See: +// https://google.github.io/styleguide/objcguide.html#import-and-include +// https://microsoft.github.io/objc-guide/Headers/ImportAndInclude.html #ifdef ORT_ENABLE_EXTENSIONS -extern "C" { -// Note: Declared in onnxruntime_extensions.h but forward declared here to resolve a build issue: -// (A compilation error happened while building an expo react native ios app, onnxruntime_c_api.h header -// included in the onnxruntime_extensions.h leads to a redefinition conflicts with multiple object defined in the ORT C -// API.) So doing a forward declaration here instead of #include "onnxruntime_extensions.h" as a workaround for now -// before we have a fix. -// TODO: Investigate if we can include onnxruntime_extensions.h here -OrtStatus *RegisterCustomOps(OrtSessionOptions *options, const OrtApiBase *api); -} // Extern C +#include "onnxruntime_cxx_api.h" +#include "onnxruntime_extensions.h" +#else +#include "onnxruntime/onnxruntime_cxx_api.h" #endif @implementation OnnxruntimeModule diff --git a/js/react_native/ios/TensorHelper.h b/js/react_native/ios/TensorHelper.h index f0936cce8b..7379492126 100644 --- a/js/react_native/ios/TensorHelper.h +++ b/js/react_native/ios/TensorHelper.h @@ -5,7 +5,19 @@ #define TensorHelper_h #import -#import + +// Note: Using below syntax for including ort c api and ort extensions headers to resolve a compiling error happened +// in an expo react native ios app (a redefinition error happened with multiple object types defined within +// ORT C API header). It's an edge case that the compiler allows both ort c api headers to be included when #include +// syntax doesn't match. For the case when extensions not enabled, it still requires a onnxruntime prefix directory for +// searching paths. Also in general, it's a convention to use #include for C/C++ headers rather then #import. See: +// https://google.github.io/styleguide/objcguide.html#import-and-include +// https://microsoft.github.io/objc-guide/Headers/ImportAndInclude.html +#ifdef ORT_ENABLE_EXTENSIONS +#include "onnxruntime_cxx_api.h" +#else +#include "onnxruntime/onnxruntime_cxx_api.h" +#endif @interface TensorHelper : NSObject