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

Replace no_elf_tls with target_thread_local #30678

Merged
merged 1 commit into from
Jan 12, 2016
Merged

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jan 2, 2016

#30417 replaced the --disable-elf-tls configure option with a cfg(target_thread_local) target attribute for thread_local!, but scoped_thread_local! still used the old cfg(no_elf_tls) option. This PR changes scoped_thread_local! to use cfg(target_thread_local) to determine whether the #[thread_local] attribute is available. It also removes the --disable-elf-tls configure option since it is no longer used.

I also re-enabled the use of #[thread_local] on AArch64. It was originally disabled in the PR that introduced AArch64 (#19790), but the reasons for this were not explained. #[thread_local] seems to work fine in my tests on AArch64, so I don't think this should be an issue.

cc @alexcrichton @akiss77

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@brson
Copy link
Contributor

brson commented Jan 5, 2016

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned brson Jan 5, 2016
@alexcrichton
Copy link
Member

Thanks! Looks like I forgot to cover scoped TLS when I changed it. Could you leave around the ./configure option? I'd want to ensure that any current build scripts continue working. Ideally there'd be a nice big deprecation warning, but that'd require some effort to implement it.

@Amanieu
Copy link
Member Author

Amanieu commented Jan 11, 2016

I added the ./configure option back, but it is now a no-op.

@alexcrichton
Copy link
Member

@bors: r+ e304fb4

Thanks!

@bors
Copy link
Contributor

bors commented Jan 12, 2016

⌛ Testing commit e304fb4 with merge 3246eae...

bors added a commit that referenced this pull request Jan 12, 2016
I also re-enabled the use of `#[thread_local]` on AArch64. It was originally disabled in the PR that introduced AArch64 (#19790), but the reasons for this were not explained. `#[thread_local]` seems to work fine in my tests on AArch64, so I don't think this should be an issue.

cc @alexcrichton @akiss77
@bors bors merged commit e304fb4 into rust-lang:master Jan 12, 2016
rillian added a commit to rillian/rust that referenced this pull request May 2, 2017
Support for disabling ELF-style thread local storage in
the standard library at configure time was removed in
pulls rust-lang#30417 and rust-lang#30678, in favour of a member in
the TargetOptions database. The new method respects
MACOSX_DEPLOYMENT_TARGET on macOS, addressing the
original use case for this configure option.

However, those commits left the configure option itself
in place. It's no longer referenced anywhere and can
be removed.
bors added a commit that referenced this pull request May 4, 2017
Remove obsolete --disable-elf-tls configure switch.

Support for disabling ELF-style thread local storage in
the standard library at configure time was removed in
pulls #30417 and #30678, in favour of a member in
the TargetOptions database. The new mentod respects
MACOSX_DEPLOYMENT_TARGET on macOS, addressing the
original use case for this configure optionl

However, those commits left the configure option itself
in place. It's no longer referenced anywhere and can
be removed.
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.

5 participants