Skip to content

Add Apple log sink.#7820

Merged
edgchen1 merged 6 commits into
masterfrom
edgchen1/apple_logging
May 27, 2021
Merged

Add Apple log sink.#7820
edgchen1 merged 6 commits into
masterfrom
edgchen1/apple_logging

Conversation

@edgchen1
Copy link
Copy Markdown
Contributor

Description
Add a log sink for Apple platforms. This version uses NSLog().

Motivation and Context
Enable logging with the Apple logging system.

@edgchen1 edgchen1 requested a review from a team as a code owner May 25, 2021 02:11
Comment thread cmake/onnxruntime_providers.cmake
Comment thread onnxruntime/core/session/ort_env.cc Outdated
} else {
#ifdef __ANDROID__
ISink* sink = new AndroidLogSink();
#if defined(__ANDROID__)
Copy link
Copy Markdown
Contributor

@guoyu-wang guoyu-wang May 25, 2021

Choose a reason for hiding this comment

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

Do we want to add an separated function to host all these #ifdef's?
#Resolved


- (void)dealloc {
[self cleanup];
[super dealloc];
Copy link
Copy Markdown
Contributor

@guoyu-wang guoyu-wang May 25, 2021

Choose a reason for hiding this comment

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

You don't need this if ARC is enabled
https://clang.llvm.org/docs/AutomaticReferenceCounting.html#dealloc

I guess we hit some pipeline failure on this
/Users/runner/work/1/s/onnxruntime/core/providers/coreml/model/model.mm:170:10: error: ARC forbids explicit message send of 'dealloc' #Resolved

Copy link
Copy Markdown
Contributor Author

@edgchen1 edgchen1 May 26, 2021

Choose a reason for hiding this comment

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

-fobjc-arc is somehow not going through in my tests, seeing a different error as a result:
error: method possibly missing a [super dealloc] call [-Werror,-Wobjc-missing-super-calls]
trying to figure that out.

@edgchen1 edgchen1 requested a review from snnn May 26, 2021 22:28
Copy link
Copy Markdown
Contributor

@skottmckay skottmckay left a comment

Choose a reason for hiding this comment

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

:shipit:

@edgchen1 edgchen1 merged commit 13622ba into master May 27, 2021
@edgchen1 edgchen1 deleted the edgchen1/apple_logging branch May 27, 2021 17:03
edgchen1 added a commit that referenced this pull request Jun 12, 2021
Add a log sink for Apple platforms. This version uses NSLog().
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