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

Remove Sized bound from scoped_thread_local! and ScopedKey #25232

Closed
wants to merge 1 commit into from

Conversation

Diggsey
Copy link
Contributor

@Diggsey Diggsey commented May 9, 2015

Fixes #25193

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@Diggsey
Copy link
Contributor Author

Diggsey commented May 9, 2015

Unfortunately it's still more limited than I'd like: this is a case where HKT would help, since the "type" of a scoped thread local, should actually be a type constructor, which would produce a concrete type given an arbitraty lifetime. However, I think it's also possible to implement the same thing using macros so that the following would be possible:

scoped_thread_local!(static TEST: &'scope u32) where 'scope is a lifetime specifier filled in by the macro. (Apparantly macro hygiene doesn't prevent this?)

This allows more complicated uses:
scoped_thread_local!(static TEST: &'scope Guard<'scope>)
which would otherwise be impossible.

The only tricky part is how to check that 'scope is only ever used contravariantly. I think not enforcing that could lead to unsafety, although I haven't come up with any examples yet.

@alexcrichton
Copy link
Member

Now that I recall the limitation that we're using OS TLS as a backend sometimes, which has the requirement that the pointer is word-sized, I'm now actually somewhat more hesitant about this. This is adding an extra layer of indirection in all cases, which may not always be desired.

I think the problems you pointed out about still being able to specify a lifetime are still relevant to this primitive, so it may be best to fix those first perhaps?

@Diggsey
Copy link
Contributor Author

Diggsey commented May 11, 2015

@alexcrichton Yes I hadn't noticed that at first either. However, without specialization or negative trait bounds it's not possible to get optimal behaviour in both cases, so I opted for an extra layer of indirection.

I did manage to implement the enhancement to "scoped_thread_local!":
rust-lang/rfcs#1112
https://github.com/Diggsey/rust/tree/scoped_thread_local_hkt

The implementation might be too much of a hack to get merged in right now (what do you think?) although mostly because of unnecessary limitations in the compiler, which would not be too much work to fix:

  1. The lack of a "lifetime token" type for use by the macro.
  2. The lack of hygiene for structs generated in a macro, combined with the inability to generate unique identifiers.

@alexcrichton
Copy link
Member

Oh wow, interesting! I do suspect, however, that that's a bit complicated to land in-tree right now. I believe that adding a lifetime token for a macro may not be too hard (would require an RFC, but seems backwards compatible), but the hygiene problem may be difficult to overcome.

@HeroesGrave
Copy link
Contributor

I opened an issue a little while back about the lack of a lifetime macro fragment. #23956

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 26, 2015
@alexcrichton
Copy link
Member

Ok for now I'm going to close this due to the need for the double-indirection in sized types, I think we may be able to explore some more macro-fun possibilities here in the future, but for now limiting this to word-sized storage is the OS limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScopedKey<T> has unnecessary implicit Sized bound
5 participants