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.

Sign up for GitHub

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

Jump to bottom

Remove GPR_*_TLS macros except PTHREAD #26974

Merged
merged 1 commit into from
Aug 12, 2021
Merged

Remove GPR_*_TLS macros except PTHREAD #26974

merged 1 commit into from
Aug 12, 2021

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Aug 11, 2021

Use c++11 thread_local when GPR_PTHREAD_TLS isn't defined.

Follow up to #26942.

@veblush cc @ctiller

@tamird
Remove GPR_*_TLS macros except PTHREAD
3b29666 
Use c++11 thread_local when GPR_PTHREAD_TLS isn't defined.
@ctiller ctiller added release notes: yes Indicates if PR needs to be in release notes area/core lang/core kokoro:force-run labels Aug 11, 2021
@tamird
Copy link
Contributor Author

tamird commented Aug 11, 2021

Are the iOS tests broken?

Wed Aug 11 14:07:22 PDT 2021 - /Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/test/cpp/ios/Pods/Headers/Public/Protobuf-C++/google/protobuf/stubs/common.h:49:10: fatal error: 'google/protobuf/stubs/stringpiece.h' file not found
Wed Aug 11 14:07:22 PDT 2021 - #include <google/protobuf/stubs/stringpiece.h>
Wed Aug 11 14:07:22 PDT 2021 -          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Wed Aug 11 14:07:22 PDT 2021 - 1 error generated.

doesn't seem like it should be affected by this change.

tamird
tamird commented Aug 11, 2021
View reviewed changes
include/grpc/impl/codegen/port_platform.h
#endif
#endif
#ifndef GPR_STDCPP_TLS
#if !(defined(__has_feature) && __has_feature(cxx_thread_local))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

one open question: I could move this feature detection directly into tls.h and be rid of GPR_*_TLS altogether. Do folks have a preference?

Copy link
Member

Choose a reason for hiding this comment

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

Keep it here... we blanket replace this file in some other environments with a hand-rolled one where we don't necessarily want feature detection (this case wouldn't hurt, but consistency matters)

@ctiller
Copy link
Member

ctiller commented Aug 12, 2021

Running tests once more and I'll probably merge in the morning.

@ctiller ctiller merged commit 96fb5d4 into grpc:master Aug 12, 2021
@tamird tamird deleted the remove-tls-gcc-msvc branch August 12, 2021 13:48
Vignesh2208 pushed a commit to Vignesh2208/grpc that referenced this pull request Aug 20, 2021
@tamird @Vignesh2208
Remove GPR_*_TLS macros except PTHREAD (grpc#26974)
aac7086 
Use c++11 thread_local when GPR_PTHREAD_TLS isn't defined.
lidizheng pushed a commit to lidizheng/grpc that referenced this pull request Sep 23, 2021
@tamird @lidizheng
Remove GPR_*_TLS macros except PTHREAD (grpc#26974)
442eb88 
Use c++11 thread_local when GPR_PTHREAD_TLS isn't defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctiller ctiller ctiller approved these changes

Assignees
No one assigned
Labels
area/core lang/core release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tamird @ctiller @grpc-kokoro