Skip to content

Remove GPR_*_TLS macros except PTHREAD#26974

Merged
ctiller merged 1 commit intogrpc:masterfrom
tamird:remove-tls-gcc-msvc
Aug 12, 2021
Merged

Remove GPR_*_TLS macros except PTHREAD#26974
ctiller merged 1 commit intogrpc:masterfrom
tamird:remove-tls-gcc-msvc

Conversation

@tamird
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@tamird tamird Aug 11, 2021

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
Copy Markdown
Member

@ctiller ctiller Aug 12, 2021

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
Copy Markdown
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.

3 participants

@tamird @ctiller @grpc-kokoro