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

Add a Share kind #12686

Merged
merged 13 commits into from
Mar 20, 2014
Merged

Add a Share kind #12686

merged 13 commits into from
Mar 20, 2014

Conversation

flaper87
Copy link
Contributor

@flaper87 flaper87 commented Mar 4, 2014

Share implies that all reachable content is threadsafe.

Threadsafe is defined as "exposing no operation that permits a data race if multiple threads have access to a &T pointer simultaneously". (NB: the type system should guarantee that if you have access to memory via a &T pointer, the only other way to gain access to that memory is through another &T pointer)...

Fixes #11781
cc #12577

What this PR will do

What's left to do in a separate PR (after the snapshot)?

  • Remove Freeze completely

@nikomatsakis
Copy link
Contributor

cc me

@brson
Copy link
Contributor

brson commented Mar 6, 2014

I'm glad to see this.

Can you make the followup change that converts some Freeze bounds to Share and removes some NoFreeze markers? Seeing the impact of this change would be enlightening.

@flaper87
Copy link
Contributor Author

flaper87 commented Mar 6, 2014

@brson sure thing, I was planning to do that as part of #12577 but I agree that doing it here may be enlightening and would help to spot some corner cases.

@flaper87
Copy link
Contributor Author

flaper87 commented Mar 7, 2014

@brson the PR is not ready but I added a new commit that replaces Freeze bounds with Share in case you want to take a look at it. :)

Weak { ptr: self.ptr,
nosend: marker::NoSend,
noshare: marker::NoShare
}
Copy link
Member

Choose a reason for hiding this comment

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

This style is more commonly found as:

Structure {
    field1: value1,
    field2: value2,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

@flaper87
Copy link
Contributor Author

@nikomatsakis r?

@alexcrichton
Copy link
Member

Travis seems to have a legitimate problem

@brson
Copy link
Contributor

brson commented Mar 16, 2014

This is ready to go and looks awesome. @nikomatsakis can you review?

@brson
Copy link
Contributor

brson commented Mar 16, 2014

I'm not sure what the effect of using Unsafe in ArcData is since ArcData is always mutated behind an unsafe pointer. If ArcData needs Unsafe then does RcBox?

@brson
Copy link
Contributor

brson commented Mar 16, 2014

Do we still intend to add a Mut<T> type on top of Unsafe<T>? Can a number of these uses of Unsafe be Mut in the future?

@nikomatsakis
Copy link
Contributor

@flaper87 r+ modulo nits. I see some Travis CI failures, though? Didn't investigate.

match expr.node {
// Just visit the expression if the
// item is taking an address.
ast::ExprAddrOf(..) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem quite right. What about something like:

struct Wrap<T> { field: T }
static UNSAFE: Unsafe<int> = ...;
static WRAPPED_UNSAFE: Wrap<&'static Unsafe<int>> = Wrap { field: &UNSAFE };

@nikomatsakis
Copy link
Contributor

I think I would give r+ just to get this landed (and do the cleanups I suggested as a follow-up), except that Travis reporting a 100+ character line error.

bors added a commit that referenced this pull request Mar 20, 2014
`Share` implies that all *reachable* content is *threadsafe*.

Threadsafe is defined as "exposing no operation that permits a data race if multiple threads have access to a &T pointer simultaneously". (NB: the type system should guarantee that if you have access to memory via a &T pointer, the only other way to gain access to that memory is through another &T pointer)...

Fixes #11781
cc #12577 

What this PR will do
================

- [x] Add Share kind and
- [x]  Replace usages of Freeze with Share in bounds.
- [x] Add Unsafe<T> #12577
- [x] Forbid taking the address of a immutable static item with `Unsafe<T>` interior

What's left to do in a separate PR (after the snapshot)?
===========================================

- Remove `Freeze` completely
@bors bors closed this Mar 20, 2014
@bors bors merged commit 7b19574 into rust-lang:master Mar 20, 2014
@flaper87 flaper87 deleted the shared branch March 22, 2014 17:42
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.

Add Share kind
6 participants