dtolnay / cxx

Safe interop between Rust and C++
https://cxx.rs
Apache License 2.0
5.89k stars 334 forks source link

Expose a nonconst reference wrapper binding #537

Open dtolnay opened 3 years ago

dtolnay commented 3 years ago

This has very interesting implications for the SharedPtr and UniquePtr API. In Rust, it's not sound to expose a safe deref from &SharedPtr<T> to &mut T / Pin<&mut T> because the lifetime system is not expressive enough to rule out calling it twice, and calling it twice produces overlapping &mut references which is UB in Rust. However, in C++ it's not, so it's safe to expose &'a SharedPtr<T> to NonConstRef<'a, T> in Rust and have NonConstRef<T> become mutable T& on the C++ side, as long as there is no conversion NonConstRef<'a, T> to &'a mut T (&'a T is fine). This is a safe way to capture the semantics of T& std::shared_ptr<T>::operator*() const noexcept.

Needs design, but possibly we can tie this to https://en.cppreference.com/w/cpp/utility/functional/reference_wrapper and piggy back on ReferenceWrapper<T>.

vi commented 3 years ago

Is using mutable keyword on C++ side a valid workaround for inability to call non-const methods behind SharedPtr?

dtolnay commented 3 years ago

Yes, that is effectively equivalent to the semantics of Cell. You'd have to make sure to follow all the same restrictions as Cell, i.e. not expose references to the mutable field which outlive the method call and not implement Sync for the enclosing Rust type.

CAD97 commented 3 years ago

Is there a reason to have this as a type rather than an attribute? I'm working with a library that uses a lot of cell-semantics nonconst referenes, and exposing them to Rust as &mut is very wrong due to the exclusivity guarantee. All I need is to be able to write something like

unsafe fn SetActorLocationAndRotation(
    #[nonconst] self: &AActor,
    NewLocation: FVector,
    // ...
) -> bool;

to drop the const-qualification. Anyone using these types is already well aware that they have Cell-semantic shared mutability, and adding any more ceremony to the reference would be needlessly verbose.

(EDIT: well, while #[nonconst] works for function arguments, it doesn't work for the return type, which is also important to be able to mark this way...)

sollyucko commented 3 years ago

Would using &Cell<T> or Pin<&Cell<T>> instead of Pin<&mut T> work? (Either explicitly written or generated by an attribute/macro.)

mattiekat commented 3 years ago

@CAD97 #936 seems relevant. Ran into something similar myself and just ended up writing wrappers.

CAD97 commented 3 years ago

I'll keep it in mind.

I'm mostly concerned about people accidentally stumbling into cases that use nonconst methods, potentially even in a non threadsafe manner (thus correctly nonconst per cxx's modern dialect of C++ it interfaces to), but which aren't compatible with Rust's exclusive ownership concept.

Especially with additional tooling such as autocxx, it's not that difficult to just use a Pin<&mut T> without realizing exactly what a restriction that puts onto the C++ side.

It's also not entirely decided what exactly exclusive ownership means w.r.t. FFI, as I understand it, especially when you have multiple layers of FFI boundaries passing &mut back and forth.

In these cases, it's much simpler to not assert language-level uniqueness on the Rust side.

It's fine for cxx to define the C++ dialect it works with as not including APIs with method receivers which can't be directly translated as &T, &mut T, or Pin<&mut T>. But the ease of getting it subtly but disastrously wrong scares me.

And at this point, I'm thinking, maybe it just should. There's no shortcut to triple checking your FFI invariants line up. cxx can automate a common subset of it, but you're still on the hook to make sure you match your FFI after adopting cxx's as well.

adetaylor commented 2 years ago

I've been playing around with this here (implementation here).

The ergonomics seem OK - no worse than Pin<&mut T> which it would be replacing. Unless I'm missing something.

More things to think about:

dcsommer commented 2 years ago

In Rust, it's not sound to expose a safe deref from &SharedPtr to &mut T / Pin<&mut T> because the lifetime system is not expressive enough to rule out calling it twice

Would building RefCell semantics into cxx::SharedPtr to enable dynamically checked borrow rules work? The C++ side would manually have to uphold the invariant to not save incompatible references while Rust code is running, but on the Rust side we could ensure safety I think. Could cxx::SharedPtr add an extra word to track borrow state and define these instead of pub fn as_ref(&self)?

pub fn borrow(&self) -> Ref<'_, T>
pub fn borrow_mut(&self) -> RefMut<'_, Pin<T>>
pub fn try_borrow(&self) -> Option<Ref<'_, T>>
pub fn try_borrow_mut(&self) -> Option<RefMut<'_, Pin<T>>>

I don't think the original pub fn as_ref(&self) -> Option<&T> can remain unfortunately, since the dynamically checked mutable borrows would not statically prevent calling as_ref(). This approach adds runtime overhead and adds memory writes where there used to only be reads. However, I think it would make the container more usable overall. I'm curious what folks think of this approach.

bsilver8192 commented 2 years ago

I don't think the original pub fn as_ref(&self) -> Option<&T> can remain unfortunately, since the dynamically checked mutable borrows would not statically prevent calling as_ref(). This approach adds runtime overhead and adds memory writes where there used to only be reads. However, I think it would make the container more usable overall. I'm curious what folks think of this approach.

Even worse than just adding writes, it adds atomic writes. And unlike RefCell, it's split across languages where compilers will have more trouble eliminating them. I can see value in having it as an option, but I don't think it's worth using it everywhere.

Enyium commented 1 year ago

I also had the case where I wanted to make functionality available that was implemented with non-const C++ member functions, but which only performed read-actions. So, the natural representations in the Rust API I created needed to have &self parameters. Thus, no mutable access to Pin<&mut T> struct fields.

I created this Rust equivalent to C++'s const_cast:

pub(crate) unsafe fn cxx_const_cast<T: UniquePtrTarget>(value: &T) -> Pin<&mut T> {
    #![inline]
    //! Presents an immutable reference as a mutable one for the purpose of calling a CXX bridge
    //! function (casts the constness away). The mutable reference must not actually be mutated!
    //! (Otherwise, bring mutability into the Rust code.)
    //!
    //! This is meant as a last resort to avoid having to write a C++ wrapper function every
    //! time some API function isn't declared as `const` on the C++ side, even though it should
    //! be. In that wrapper, the same thing would be done with a C++ `const_cast<...>(...)`
    //! anyway.

    #[allow(clippy::cast_ref_to_mut)]
    Pin::new_unchecked(&mut *(value as *const T as *mut T))
}

It worked well. Later, I discovered this warning, however, which I don't fully understand, but which seems to be very relevant:

The assumption "If you run this... you get a segfault" suggests that the compiler is actually bound to compile the code to something meaningful, but incorrect given the values provided at runtime. In fact, very_bad_function may not be compiled to anything meaningful at all, as the optimizer may make assumptions about reachability based on the non-aliasing of &mut T. Breaking aliasing rules is not an extra reason "in addition to" the problem of writing to read-only memory -- it's the only reason why this code has undefined behavior...

https://stackoverflow.com/a/54242058/10749231

Perhaps, this could be solved by preventing such compiler optimizations?


I now have no need for the function anymore. I had a point where more and more lifetimes came into play with the Pin<&mut T>s I had in my structs, which brought be me into Pin hell. I viewed handling these lifetimes to be a very unnecessary hassle and shifted to using *mut T where T: UniquePtrTarget and a technique I call just-in-time pinning where I transform *mut T into either &T or Pin<&mut T> as the CXX bridge requires it.