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

Unnecessary transmutes #27

Open
ebfull opened this issue Oct 21, 2015 · 3 comments
Open

Unnecessary transmutes #27

ebfull opened this issue Oct 21, 2015 · 3 comments

Comments

@ebfull
Copy link

ebfull commented Oct 21, 2015

Are the transmutes necessary for performance reasons? Chan could be reconstructing itself as it takes itself by value, and should not need to transmute. I'm reasonably confident LLVM will optimize this away.

@laumann
Copy link
Collaborator

laumann commented Oct 29, 2015

The transmutes are not necessary, no. What you mean is that we could do

impl<E, P, A: marker::Send + 'static> Chan<E, Send<A, P>> {
    pub fn send(self, v: A) -> Chan<E, P> {
        unsafe {
            write_chan(&self, v);
        }
        let Chan(tx, rx, _) = self;
        Chan(tx, rx, PhantomData)
    }
}

instead?

We tried it out at some point, and it works just the same - I think we kept the transmutes, because we were not convinced that there was any difference between the two, and thought that the transmute is a "clearer" statement of intention (that we're changing the type).

@Munksgaard
Copy link
Owner

and thought that the transmute is a "clearer" statement of intention (that we're changing the type).

Perhaps we should consider actually making the types explicit every time we use transmute?

@ebfull
Copy link
Author

ebfull commented Nov 14, 2015

Well, transmutes should be avoided simply because they require unsafe { }, and wouldn't you want to reduce the amount of unsafe code? :) (Of course, changing the session type with reckless disregard is itself unsafe potentially because it breaks the expectations elsewhere, so you should ensure the session type cannot be changed outside the library at least. This might just lead you back to unsafe to begin with, though.)

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

No branches or pull requests

3 participants