[WIP] Forbid object lifetime changing pointer casts by BoxyUwU · Pull Request #136776 · rust-lang/rust (original) (raw)

I'll try not to re-cover what other people have already commented, e.g. feelings about teachability here, or pointer casts not being strictly more powerful (or equal to) transmutes.

First, reading the lang meeting minutes and the summarizing comment here I get the impression that lang is making decisions under the belief that writing *const dyn Trait means *const dyn Trait + 'static in all circumstances. This is not true.

In item signatures we have the dyn type lifetime default rules that do give this behaviour, e.g. static FOO: *const dyn Trait = ...; is equivalent to *const dyn Trait + 'static. However, in bodies (e.g. the initializer of a static, or inside the { ... } of a function), elided lifetimes of dyn types are simply unconstrained lifetimes that are inferred by the borrow checker. Example:

fn foo<T: Trait>(ptr: *const T) { let a: *const dyn Trait = ptr; }

This currently compiles on stable and will continue to do so under this PR. If the type annotation on the let statement meant *const dyn Trait + 'static then this would not compile even on stable.

I am somewhat confused by the proposal to start linting on code. The exact details of what we're supposed to lint on are unclear to me, especially given the prior context of having already ruled out being able to do a FCW. Having read the meeting notes my understand is that lang is considering a lint that forbids eliding dyn type lifetimes altogether in as casts?

For example that the following would emit a lint:

fn foo<T: Trait>(ptr: *const T) { ptr as *const dyn Trait; }

I would expect this to have a lot of false positives. I also don't believe this really helps alleviate the footguns involved with these pointer casts (which seemed to be a big point of focus in the meeting notes, that this removes a footgun). Even when explicitly writing out the lifetimes involved you can just write lifetimes that make it seem like no real lifetime changing has occurred.

Taking the example from the lang meeting notes:

fn foo(output: &dyn Write) { /* ... */ unsafe { output as *const (dyn Write + 'static) as mut (dyn Write + 'static) }; / ... */ }

This example still seems like a footgun, the explicitly written lifetime bounds almost make it worse as you could easily believe that the pointee of output lives for 'static and that we then perform a pointer cast which does not change the lifetime bound.

In order to avoid a footgun here the type of output with the lifetime bound written explicitly needs to be present so that you can see the source and target types of the cast. e.g. my_ref as &(dyn Trait + 'a) as *const dyn Trait + 'static doesn't seem like a footgun as you can tell that a cast from + 'a to + 'static is occuring.

I am also unclear on where lang stands in regards to breaking such unsafe code over an edition. The summary comment here was not particularly clear, and reading the meeting notes I have been given the impression that no real consensus was arrived at and that the hypothetical lint obviates any need to break this unsafe code.

I am not sure if things have changed for lang now given my previous statements about footguns and dyn-type elision rules.

Before going through with this I would like for lang to fully commit to either supporting such casts as an actual language feature, or only as a migration hack with a commitment to break it going forward across the 2025 edition (with the understanding that it may not be possible to FCW or auto-fix).

Generally I would like to avoid going forward with this with a vague handwaving of "we could potentially break this over an edition" to assuage concerns of teachability/etc, and then potentially wind up with lang deciding not to do so.

On the general consistency of pointer casts here. Lang has already forbidden casting *const dyn Trait to *const dyn Trait + Send, and have also already forbidden casting *const dyn Trait<'a> to *const dyn Trait<'b> (or even *const dyn Trait<T> to *const dyn Trait<U>).

If lang wishes to extend the power of as casts in unsafe code to be able to do more "transmute like" casts as a language feature, then I would expect us to also allow these arbitrary casts of dyn types. Is this something lang is on board with?

On the other hand if this is only intended as a migration strategy then this inconsistency seems fine to me as we expect all code to either be legacy (i.e. no longer maintained) or updated to a newer edition where there is no inconsistency.

Finally, reading the meeting notes I see that there were parallels drawn between raw pointer derefs being the same syntax as derefing references and how one is safe and the other not.

Admittedly I do see the parallel here but I think it's worth pointing out that it is significantly easier to determine whether one is dereferencing a pointer or reference, in comparison to determining whether a lifetime has been extended. I think lang is aware of this hence the discussion of the lint and avoiding footguns? I think this is worth revisiting in light of previous statements about footguns/the lint.

I would also like to note that r-a currently has the ability to highlight unsafe operations in a specific colour to make it more clear when safety invariants are introduced in unsafe code. I don't know how well that can be supported with this change where all as casts of pointers to dyn-types in unsafe blocks are sometimes morally-unsafe but only determinable by the specific lifetimes in play.

Do we expect r-a to be able to figure this out or just conservatively consider all of these as casts to be unsafe operations? Similar to the previous section of my comment, I don't think this is much of a problem if this change is only done as a migration method and broken over an edition. It's probably fine if legacy codebases have "overly liberal" application of "this operation is unsafe" highlighting.

I generally find it hard to go along with the idea that its "just" allowing a new operation to be performed inside of unsafe code rather than "changing semantics/disabling checks" when both humans and tooling is going to struggle to even distinguish these two different operations. This to the extent that the compiler impl would literally be to unconditionally generate the "unsafe transmute cast" operation in unsafe blocks rather than the checked kind.

I think the fact that there is ~no world in which the compiler impl will align with this mental model of "its a new operation allowed in unsafe" should be a good signal that it's not the right mental model.