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

Adding support for regions in unsafe pointers #7967

Closed
wants to merge 13 commits into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Jul 22, 2013

This patch series fixes #7694. The main reason for this is that without this patch it's trivial to have an interior pointer to a rust structure live past that structure's deallocation. I go into more detail in #7694 if you are curious. The end result is that if you really do know more than the borrow checker, you now need to use cast::transmute to convert a *'a T into a *'static T if you really want a leakable pointer.

This patch isn't quite up for commital yet though. First, it still needs to be merged into head. Second, it needs a snapshot dance in order to first allow us to parse *'a T, then another to actually implement the regions.

cc @nikomatsakis, as you are probably the best one to review this.

@erickt
Copy link
Contributor Author

erickt commented Jul 23, 2013

Updated to be rebased on HEAD. This needs a snapshot made on commit a972633. Then the rest can be merged in.

@nikomatsakis
Copy link
Contributor

This seems like something we will need to discuss at the meeting, as well!

@brson
Copy link
Contributor

brson commented Jul 27, 2013

Agree we should discuss this.

@erickt
Copy link
Contributor Author

erickt commented Jul 29, 2013

I've thought about this a bit more (see this thread for details), and now I'm starting to think we can completely move unsafe pointers out of the compiler and into a library. If I understand things correctly, this code snippet should have the same semantics as my unsafe region pointers:

mod intrinsics {
    extern {
        fn get(&self) -> T;
        fn set(&mut self, value: T);
        fn memcpy<T>(dest: &mut RawPtr<T>, src: &RawPtr<T>, len: uint);
        ...
    }
}

struct RawPtr<T>;

impl<T> RawPtr<T> {
    unsafe fn get(&self) -> T { intrinsics::ptr_get(self) }
    unsafe fn set(&mut self, value: T) { intrinsics::ptr_set(self, value) }
    fn is_null(&self) -> bool {
        unsafe {
            let x: uint = cast::intrinsics(self);
            x == 0
        }
    }
    fn offset<'a>(&'a self, len: uint) -> &'a RawPtr<T> { ... }
    ...
}

fn as_imm_ptr<'a, T>(v: &'a [T]) -> &'a RawPtr<T> {
    let (ptr, _): (&'a RawPtr<T>, uint) = cast::transmute(v);
    ptr
}

mod libc {
    extern {
        fn getenv(&RawPtr<libc::c_char>) -> &'static RawPtr<libc::c_char>;
}

fn getenv(s: &str) -> Option<~str> {
    unsafe {
        let charp = s.as_c_str(|ptr| libc::getenv(ptr));
        if charp.is_null() {
            None
        } else {
            str::from_c_str(charp)
        }
    }
}

...

@erickt
Copy link
Contributor Author

erickt commented Aug 5, 2013

I had a long conversation with @graydon about this, and he's still not in favor of this proposal. My intention was to make the process of taking a rust structure and passing it to a C function safer, but he thought we'd be implying that the C function would respect our regions, but that's not necessarily the case. He went on further to say that #3511 should help address my concern of passing an interior pointer of a deleted temporary to a function.

@nikomatsakis / @brson: have either of you formed an opinion about this? I'm tempted to close this PR, but before I do I'd like to hear if either of you feel it's worth pursuing this idea.

@thestinger
Copy link
Contributor

@erickt: would #5922 be anything close to what you want?

@thestinger
Copy link
Contributor

I'm going to close this for now, since it requires a rebase and there doesn't seem to be support for handling the issue this way.

@thestinger thestinger closed this Aug 16, 2013
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.

Auto-coercing from &T to *T is unsafe
4 participants