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

Atomic function pointers #2481

Open
gnzlbg opened this Issue Jun 20, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 20, 2018

Following the discussion in rust-lang/rust#51559 I am opening this to track the lack of atomic function pointers in the std library (and the ecosystem in general).

We currently have std::sync::atomic::AtomicPtr but it does not work with function pointers by design:

fn foo() {}
static Foo: AtomicPtr<fn()->()> = AtomicPtr::new(foo);

emits

error[E0308]: mismatched types
 --> src/main.rs:5:50
  |
5 | static Foo: AtomicPtr<fn()->()> = AtomicPtr::new(foo);
  |                                                  ^^^ expected *-ptr, found fn item
  |
  = note: expected type `*mut fn()`
             found type `fn() {foo}`

This is at the pure brainstorming stage, but enabling this use case is probably going to need an AtomicFnPtr or similar type unless we retrofit AtomicPtr to also work with fn items.


EDIT: relevant threads/rfcs:

[0] pre-RFC: Extended atomic types proposes a generic atomic type that uses constraints to be generic. I don't know if that approach can be feasible extended to function pointer types.

[1] RFC 1543 (merged): Add more integer atomic types. Only mentions AtomicPtr, but it does not mention pointers to functions.

[2] Function pointers can be stored in atomic usizes just fine.

@Amanieu

This comment has been minimized.

Copy link
Contributor

Amanieu commented Jun 20, 2018

The main issue that I can see here is that we would need variadic generics to make this work for all possible argument lists for the inner function type.

Also have you considered whether AtomicFnPtr should allow a "null" value? In which case it would wrap a Option<fn()> rather than a raw fn().

@rkruppe

This comment has been minimized.

Copy link
Member

rkruppe commented Jun 20, 2018

Variadic generics aren't even enough, since fn types can have higher rank lifetimes (e.g., fn(&str) -> &str is actually a for<'a> ... binder). That issue isn't specific to atomics (e.g., it also affects trait impls for function pointers, except the magic compiler-generated ones), so there's motivation for some way to abstract over those HRLBs, but I don't know if that's even really possible and I certainly haven't heard of any proposals for how to add it to Rust.

@gnzlbg

This comment has been minimized.

Copy link
Contributor Author

gnzlbg commented Jun 20, 2018

The main issue that I can see here is that we would need variadic generics to make this work for all possible argument lists for the inner function type.

Thinking of: fn<'a, 'b, T, U>(x: &'a T, y: &'b U) -> &'a T vs fn<'a, T, U>(x: &'a T, y: &'a U) -> &'a T vs fn<'a, T, U>(x: &'a T, y: U) -> &'a T, would variadic generic be enough?

Also have you considered whether AtomicFnPtr should allow a "null" value?

I think "null" is a perfectly valid value for a raw fn pointer, but I don't know if it is possible to have one in Rust: let x: fn()->() = std::ptr::null(); fails to compile.

@rkruppe

This comment has been minimized.

Copy link
Member

rkruppe commented Jun 20, 2018

fn types are non-null. Unsafely type-punning a null pointer to a fn is instant UB.

@gnzlbg

This comment has been minimized.

Copy link
Contributor Author

gnzlbg commented Jun 20, 2018

Then I think we need AtomicFnPtr<Option<fn()->()>> . . .

@Amanieu

This comment has been minimized.

Copy link
Contributor

Amanieu commented Jun 20, 2018

At this point you might as well just use my crate.

@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Oct 14, 2018

Ping @gnzlbg; what's the state of this?

( You can do: AtomicPtr::new(foo as *mut _) )

@Centril Centril added the T-libs label Oct 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.