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

Utilize PhantomData to enforce !Sync and !Send field. #35426

Merged
merged 1 commit into from
Aug 9, 2016

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Aug 6, 2016

No description provided.

@nagisa
Copy link
Member

nagisa commented Aug 6, 2016

cc @alexcrichton, a nice optimisation. Some backward compat hazard if anybody does transmute out of these, but I don’t think that would ever be problematic.

@alexcrichton
Copy link
Member

@bors: r+ 404e31b4e7f07031843bdfdba4d09e0b18eb1085

Looks good to me, thanks!

@bors
Copy link
Contributor

bors commented Aug 7, 2016

⌛ Testing commit 404e31b with merge 78cdb1e...

@bors
Copy link
Contributor

bors commented Aug 7, 2016

💔 Test failed - auto-linux-64-cross-freebsd

@frewsxcv
Copy link
Member Author

frewsxcv commented Aug 7, 2016

Compile errors fixed in the latest force push.

diff --git a/src/libstd/sys/unix/os.rs b/src/libstd/sys/unix/os.rs
index bc4c22a..6b8dfa5 100644
--- a/src/libstd/sys/unix/os.rs
+++ b/src/libstd/sys/unix/os.rs
@@ -400,7 +400,7 @@ pub fn args() -> Args {
         }
     }

-    Args { iter: res.into_iter(), _dont_send_or_sync_me: ptr::null_mut() }
+    Args { iter: res.into_iter(), _dont_send_or_sync_me: PhantomData }
 }

 #[cfg(any(target_os = "linux",
@@ -419,7 +419,7 @@ pub fn args() -> Args {
     let v: Vec<OsString> = bytes.into_iter().map(|v| {
         OsStringExt::from_vec(v)
     }).collect();
-    Args { iter: v.into_iter(), _dont_send_or_sync_me: ptr::null_mut() }
+    Args { iter: v.into_iter(), _dont_send_or_sync_me: PhantomData }
 }

 pub struct Env {

@frewsxcv frewsxcv force-pushed the os-sys-env-args-phantoms branch from 404e31b to 28218be Compare August 7, 2016 13:06
@alexcrichton
Copy link
Member

@bors: r+ 28218be

@bors
Copy link
Contributor

bors commented Aug 9, 2016

⌛ Testing commit 28218be with merge f013914...

bors added a commit that referenced this pull request Aug 9, 2016
Utilize `PhantomData` to enforce `!Sync` and `!Send` field.

None
@apasel422
Copy link
Contributor

Out of curiosity, is there a reason that the following doesn't work?

struct Args {
    iter: vec::IntoIter<OsString>,
}

struct Env {
    iter: vec::IntoIter<(OsString, OsString)>,
}

impl !Send for Args {}
impl !Sync for Args {}

impl !Send for Env {}
impl !Sync for Env {}

It ends up giving a better error message when you try to use these types in a Send or Sync context because it doesn't mention the types' internal fields.

@alexcrichton
Copy link
Member

@apasel422 yeah I think that'd work but impl !Foo is unstable right now and I just personally tend to lean towards stable before unstable things.

@bors bors merged commit 28218be into rust-lang:master Aug 9, 2016
@frewsxcv frewsxcv deleted the os-sys-env-args-phantoms branch August 9, 2016 21:19
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