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

No aliasable, non-nullable, mutable pointers #13194

Closed
emberian opened this issue Mar 29, 2014 · 15 comments · Fixed by #19765
Closed

No aliasable, non-nullable, mutable pointers #13194

emberian opened this issue Mar 29, 2014 · 15 comments · Fixed by #19765

Comments

@emberian
Copy link
Member

In this situation:

// type jit_context_t = *mut Struct__jit_context;
pub struct Context {
    priv jcx: ffi::jit_context_t
}

impl Context {
    /// Create a new Context. Returns None on out-of-memory (as libjit does)
    pub fn new() -> Option<Context> {
        unsafe {
            ffi::jit_init();
            let x: ffi::jit_context_t = ffi::jit_context_create();
            if x == null() {
                None
            } else {
                Some(Context { jcx: x })
            }
        }
    }
}

Ideally I'd be storing jcx as some non-nullable pointer, such that Option<Context> could be a single word. Unfortunately there is no type usable today. Option<*mut T> is 16 bytes on x86_64, and will always be at least one byte more than just the pointer. Option<~T> will be 8 bytes, but needs handling about the destructor and also has aliasability guarantees, which will be invalidated by the underlying libjit code. Option<&'static mut T> also has aliasing concerns, on top of nasty lifetime hack.

There is no pointer type usable to get zero overhead in this. Given the decision on #10571 (which I agree with!) it would be nice to have something usable here. Perhaps even a #[not_null] attribute, per-field, could be usable?

@emberian
Copy link
Member Author

cc @nikomatsakis @thestinger @Aatch @glaebhoerl @pnkfelix (vocal participants in #10571)

@emberian
Copy link
Member Author

(In fact, ideally, that conditionally could just be written transmute(Context { jcx: x }), though some may see that as more evil than I do)

@huonw
Copy link
Member

huonw commented Mar 29, 2014

If we expose something like #9546 as an annotation, we can write this in a library (presumably).

@emberian
Copy link
Member Author

This can't really use a non-raw pointer though because of the aliasing and mutability guarantees we have. The only type that semantically works here is *mut.

@emberian
Copy link
Member Author

And specifically adt needs to care about this (it's the Option optimization), not just LLVM.

@flaper87
Copy link
Contributor

cc @flaper87

@alexcrichton
Copy link
Member

This is a general issue with smart pointers. Both Option<Rc<T>> and Option<Arc<T>> are a word larger than they need to be. Sadly, the compiler knows a lot more about ~T than it does about Rc<T>, as in the compiler can't guarantee that the pointer inside Rc<T> is never null. (food for thought)

@emberian
Copy link
Member Author

If there's no opposition to a #[not_null] attribute here, I will write up an RFC.

@Aatch
Copy link
Contributor

Aatch commented Mar 29, 2014

I'm not against a #[not_null] attribute as, like many attributes, it shouldn't noticeably change the semantics of the type, it should strictly be an optimization (similar to #[unsafe_no_drop_flag]).

@sfackler
Copy link
Member

Maybe #[unsafe_not_null] in the spirit of #[unsafe_no_drop_flag] to give users a heads up that they need to make sure that they preserve the right invariants?

@emberian
Copy link
Member Author

Sure, I don't really care about the name.

On Sat, Mar 29, 2014 at 7:26 PM, Steven Fackler notifications@gh.neting.ccwrote:

Maybe #[unsafe_not_null] in the spirit of #[unsafe_no_drop_flag] to give
users a heads up that they need to make sure that they preserve the right
invariants?


Reply to this email directly or view it on GitHubhttps://github.com//issues/13194#issuecomment-39012516
.

http://octayn.net/

@huonw
Copy link
Member

huonw commented Mar 30, 2014

(I feel like this issue is very similar to #7576, at least in its consequences.)

@nikomatsakis
Copy link
Contributor

I think this is #7576, isn't it?

@emberian
Copy link
Member Author

Well I don't specifically care about library smart pointers, but it's
similar. (My use-case is idiomatic zero-overhead FFI)

On Mon, Mar 31, 2014 at 10:55 AM, Niko Matsakis notifications@gh.neting.ccwrote:

I think this is #7576 #7576,
isn't it?


Reply to this email directly or view it on GitHubhttps://github.com//issues/13194#issuecomment-39097739
.

http://octayn.net/

@emberian
Copy link
Member Author

emberian commented Apr 6, 2014

rust-lang/rfcs#36

bors added a commit that referenced this issue Dec 29, 2014
This extends the nullable enum opt to traverse beyond just the first level to find possible fields to use as the discriminant. So now, it'll work through structs, tuples, and fixed sized arrays. This also introduces a new lang item, NonZero, that you can use to wrap raw pointers or integral types to indicate to rustc that the underlying value is known to never be 0/NULL. We then use this in Vec, Rc and Arc to have them also benefit from the nullable enum opt.

As per rust-lang/rfcs#499 NonZero is not exposed via the `libstd` facade.

```
x86_64 Linux:
                        T       Option<T> (Before)      Option<T> (After)
----------------------------------------------------------------------------------
Vec<int>                24          32                      24
String                  24          32                      24
Rc<int>                 8           16                      8
Arc<int>                8           16                      8
[Box<int>, ..2]         16          24                      16
(String, uint)          32          40                      32
```

Fixes #19419.
Fixes #13194.
Fixes #9378.
Fixes #7576.
flip1995 pushed a commit to flip1995/rust that referenced this issue Aug 8, 2024
fix-typos

fixes some typos in lintcheck

changelog: None
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 a pull request may close this issue.

7 participants