Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an error when compiling a watchOS target not supporting Thread-local storage. #5188

Merged
merged 5 commits into from
Feb 8, 2022

Conversation

dianaafanador3
Copy link
Contributor

@dianaafanador3 dianaafanador3 commented Jan 24, 2022

Fix an error when compiling a watchOS target not supporting Thread-local storage. (#7623)

What, How & Why?

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)

@@ -168,7 +168,7 @@ bool SimulatedFailure::do_check_trigger(FailureType failure_type) noexcept
return false;
}

#if REALM_ARCHITECTURE_X86_32 && REALM_IOS
#if (REALM_ARCHITECTURE_X86_32 && REALM_IOS) || REALM_WATCHOS
Copy link
Contributor

Choose a reason for hiding this comment

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

This is related to #5102. I am going to make a guess and say that we should probably include REALM_TVOS as well. Alternatively, we already have a RLM_NO_THREAD_LOCAL which we could move to features.h so it could be used here as well.

Copy link
Contributor Author

@dianaafanador3 dianaafanador3 Jan 24, 2022

Choose a reason for hiding this comment

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

The second option will help to avoid this to happen in other targets that we are not counting on, so will be a better solution to this issue. I can do it on this PR if we agree on this approach.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this approach but seems that it doesn't work for this particular scenario

Copy link
Member

Choose a reason for hiding this comment

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

RLM_NO_THREAD_LOCAL is equal to REALM_PLATFORM_APPLE, which doesn't seem like a correct definition.

Copy link
Member

Choose a reason for hiding this comment

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

Is thread-local storage actually not supported at all on watchOS? I would have expected it to be supported on 64-bit watchOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It Is not supported on the simulator, device architecture should be arm64_32 which I just read should support it
I could just check for simulator architecture
(REALM_ARCHITECTURE_X86_64 && REALM_WATCHOS)
or just for for architecture
REALM_ARCHITECTURE_X86_64
I'm not sure if the availability only depends of the architecture and/or platform.

@dianaafanador3 dianaafanador3 merged commit e95ba38 into master Feb 8, 2022
@dianaafanador3 dianaafanador3 deleted the dp/watchos_target_not_thread_local branch February 8, 2022 11:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants