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

Tracking Issue for auto traits (auto_traits) -- formerly called opt-in built-in traits (optin_builtin_traits) #13231

Open
18 of 24 tasks
flaper87 opened this issue Mar 31, 2014 · 45 comments
Labels
A-traits Area: Trait system A-typesystem Area: The type system B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-auto_traits `#![feature(auto_traits)]` P-medium Medium priority S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@flaper87
Copy link
Contributor

flaper87 commented Mar 31, 2014

This is the tracking issue for RFC 19.

Checklist

Here is a check-list of code to write and tricky scenarios to be sure we handle:

@flaper87 flaper87 self-assigned this Mar 31, 2014
@flaper87 flaper87 changed the title opt-in built-in trait bounds RFC tracker opt-in built-in bounds traits RFC tracker Mar 31, 2014
@pnkfelix
Copy link
Member

pnkfelix commented Apr 3, 2014

Marking 1.0, P-backcompat-lang.

@nikomatsakis
Copy link
Contributor

cc me

@flaper87
Copy link
Contributor Author

I'm actively working on this. Unfortunately, I've moved quite slowly because of other bugs I had to fix. I hope to be able to submit a PR this week.

@dtolnay
Copy link
Member

dtolnay commented Jan 19, 2023

#107082 proposes an adjustment to the coherence rules involving autotraits.

@dtolnay dtolnay added T-types Relevant to the types team, which will review and decide on the PR/issue. S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. and removed E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. I-needs-decision Issue: In need of a decision. S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. labels Jan 22, 2023
@dtolnay
Copy link
Member

dtolnay commented Jan 22, 2023

I am adding a note to the checklist that the interaction between auto traits and str needs to be revisited as part of any eventual stabilization.

Currently with the following code:

auto trait AutoTrait {}

impl<T> !AutoTrait for [T] {}

we get str implements AutoTrait but [u8] does not.

Under a different libcore-based str library implementation something like #70911, we'd get that neither implements AutoTrait.

If we want to ever be able to push something like #70911 through in the future, there may need to be str-specific special cases in the autotrait feature for now.

compiler-errors added a commit to compiler-errors/rust that referenced this issue Feb 25, 2023
…or-auto, r=lcnr

Treat `str` as containing `[u8]` for auto trait purposes

Wanted to gauge `@rust-lang/lang` and `@rust-lang/types` teams' thoughts on treating `str` as "containing" a `[u8]` slice for auto-trait purposes.

`@dtolnay` brought this up in rust-lang#13231 (comment) as a blocker for future `str` type librarification, and I think it's both a valid concern and very easy to fix. I'm interested in actually doing that `str` type librarification (rust-lang#107939), but this probably should be considered in the mean time regardless of that PR.

r? types for the impl, though this definitely needs an FCP.
compiler-errors added a commit to compiler-errors/rust that referenced this issue Feb 25, 2023
…or-auto, r=lcnr

Treat `str` as containing `[u8]` for auto trait purposes

Wanted to gauge `@rust-lang/lang` and `@rust-lang/types` teams' thoughts on treating `str` as "containing" a `[u8]` slice for auto-trait purposes.

`@dtolnay` brought this up in rust-lang#13231 (comment) as a blocker for future `str` type librarification, and I think it's both a valid concern and very easy to fix. I'm interested in actually doing that `str` type librarification (rust-lang#107939), but this probably should be considered in the mean time regardless of that PR.

r? types for the impl, though this definitely needs an FCP.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 26, 2023
…or-auto, r=lcnr

Treat `str` as containing `[u8]` for auto trait purposes

Wanted to gauge ``@rust-lang/lang`` and ``@rust-lang/types`` teams' thoughts on treating `str` as "containing" a `[u8]` slice for auto-trait purposes.

``@dtolnay`` brought this up in rust-lang#13231 (comment) as a blocker for future `str` type librarification, and I think it's both a valid concern and very easy to fix. I'm interested in actually doing that `str` type librarification (rust-lang#107939), but this probably should be considered in the mean time regardless of that PR.

r? types for the impl, though this definitely needs an FCP.
@dhardy
Copy link
Contributor

dhardy commented Jun 30, 2023

This RFC is a little dated and concerns multiple topics ("auto trait", other automatically-implemented marker traits, unsafe traits). Lets summarise the RFC:

Motivation

To paraphrase (with comments):

  1. Send and Share (is this the old Sync?) should not be opt-in. This issue is solved.
  2. Move Send and Share out of the language into the standard library.
  3. Allow user-defined classification traits. Suggested uses are: "tainted", Snapshot, NoManaged, NoDrop (now !Drop).

Detailed design

Default and negative impls

We add a notion of a default impl, written:

impl Trait for .. { }

(This is now just auto trait Trait {} IIUC.)

Negative impls are only permitted if Trait has a default impl.

This is reliant on having a specified "default impl", as above (not just impl<T> Trait for T {} and not the type of default impl used for specialization). If an auto-trait is defined as exactly "a trait which is implemented by default, with opt-out", then auto trait Trait {} suffices.

Intuitively, to check whether a trait Foo that contains a default impl is implemented for some type T, we first check for explicit (positive) impls that apply to T. If any are found, then T implements Foo. Otherwise, we check for negative impls. If any are found, then T does not implement Foo. If neither positive nor negative impls were found, we proceed to check the component types of T (i.e., the types of a struct's fields) to determine whether all of them implement Foo. If so, then Foo is considered implemented by T.

Aha, the (informal) definition of an auto trait.

Modeling Send and Share using default traits

The RFC details how to define Send and Share as auto traits (the current Sync is slightly different, but still an auto trait).

(The private trait Freeze is another auto trait with some explicit opt-in and opt-out impls.)

(auto trait Unpin is another public auto trait with some explicit opt-ins. As a work-around for not having stable negative impls, Unpin has a negative impl for public type PhantomPinned.)

The Copy and Sized traits

Copy is just an ordinary (opt-in) marker trait.

Sized looks like an ordinary trait but the compiler provides all impls (no opt-in or opt-out possible).
(The same is true for trait Unsize<T: ?Sized>.)


My thoughts follow.

"auto trait" in conjunction with negative impls are a niche solution to moving a few trait definitions from the language to the standard library:

  • Send (user-defined opt-in and opt-out not allowed)
  • Sync (user-defined opt-in and opt-out not allowed)
  • Freeze (not public)
  • Unpin (user-defined opt-out allowed via an alternative mechanism)

"auto trait" does not allow all "automatically defined traits" to be offloaded from the compiler: e.g. Sized, Unsize<T>, Drop.

The RFC mentions a few other uses of auto traits:

  • "tainted" data to be excluded from some generic routine (I do not understand the motivation)
  • Snapshot, a marker such that a type T: Clone + Snapshot indicates that a clone preserves value (i.e. that the clone may not be mutated by some internal reference); this is documentation-via-type-system and possibly has some utility though I don't see how such a bound would be required (the preferred method of preserving generic data will often be to serialize, which allows writing to an external medium)
  • NoManaged
  • NoDrop. This is !Drop (also known as linear types) and explored elsewhere

The only other possible usage I can think of:

  • bytemuck::Pod: but this does not work since Pod types must not have padding

Hence it remains unclear whether there is sufficient utility for "auto traits".

The other angle for auto-traits is as an opt-out marker trait (i.e. negative impls #68318). These do not require any rules pertaining to component types, thus, for simplicity, should not use "auto traits" as defined by RFC 19.

My hesitant recommendation

Do not make "auto traits" a public feature and close this issue. They have recognised utility within the standard library but likely very little utility otherwise. (Perhaps in the future a more general form of blanket impls able to emulate the logic of auto traits will be possible. In the mean-time, I simply don't think auto traits have sufficient utility to become a fully fledged language feature.)

Possibly rename the existing "auto traits" to allow the term to be used for a simple "opt-out" trait for use with negative implementations (but this would be the topic of negative impls or a whole new RFC).

@madsmtm
Copy link
Contributor

madsmtm commented Jun 30, 2023

To me, it mostly has value for checking whether the types involved in a closure satisfy a specific property. A perfect example of this, outside the usual Send/Sync, is UnwindSafe (however badly named that is).

Concretely, I'm using it (behind a feature flag) to make autorelease pools in Objective-C sound, just as a data point for how it's useful.

@dhardy
Copy link
Contributor

dhardy commented Jun 30, 2023

Thanks for the examples, adding motivation for this feature.

The aspects of this feature I'm least comfortable with are:

  • Using a simple name like "auto trait" for what is a complex feature ("auto recursive trait"?)
  • The idea of using it as a key building block of negative impls. Potentially that feature should introduce another form of opt-out trait. (Possibly not even in the base RFC; there's some utility in negative impls without negative reasoning.)

@scottmcm
Copy link
Member

My best recollection of current lang consensus is that S-tracking-perma-unstable is correct here. This is essential for the standard library, but the ecosystem effects of "you need to know to opt out of dozens of random auto traits created by various crates" seems unworkable for a crate ecosystem, so without some solution to that, these likely will remain a standard library exclusive privilege.

@fogti
Copy link
Contributor

fogti commented Sep 22, 2023

how about coupling the auto traits so that they only get auto-used when the crate defining them is a direct dependency of the current crate (so no automatic spill of auto traits happens)? (making it crate-level opt-in, trait-level opt-out instead of pure opt-out)

@GoldsteinE
Copy link
Contributor

GoldsteinE commented Sep 27, 2023

I think pyo3 usecase wasn’t mentioned here, so I’ll bring it as a data point.

pyo3 needs to track which types can only be accessed when Python’s GIL is held. Currently it uses Send as a proxy for this property, but it’s neither neccessary nor sufficient requirement and only works as a rough heuristic. It seems like there’s no good way to do it properly without auto traits (+ negative impls). Similar usecase was mentioned above with ObjC, so it seems like this feature is neccessary for sound FFI at least in some cases.

I think that this is an evidence that auto traits currently have usecases outside of standard library. Maybe these usecases could be covered by another feature, maybe they’re not enough to sway the decision, but I think it should be recognized that auto traits have utility outside of standard library, and making them perma-unstable will probably make some usecases perma-unsound, unless some other solution is found. I feel like if this issue is to be closed, this tradeoff (“we’re OK with these cases having no known sound solution”) should be explicitly made in the final decision.

https://docs.rs/pyo3/latest/pyo3/marker/trait.Ungil.html — trait in pyo3 that should be auto
PyO3/pyo3#2141 — issue in the pyo3 repo

@GoldsteinE
Copy link
Contributor

GoldsteinE commented Sep 27, 2023

I’d also like to address this point specifically wrt Ungil:

you need to know to opt out of dozens of random auto traits created by various crates

Opting out of Ungil would only be sensible when you’re doing something with Python and are already using pyo3. In my intuition, auto traits are used to represent some properties related to a certain system (e.g. Send and Sync represent properties related to threads). You opt out of them when your type has some special properties in this system (e.g. is safe to access from multiple threads). Opting out of an auto trait is thus only sensible if you’re actually using this system, so you can promise to uphold some invariants. Opting out of auto traits related to systems you’re not actively integrating with feels like a wrong thing to do in almost any situation.

@madsmtm
Copy link
Contributor

madsmtm commented Sep 27, 2023

Opting out of auto traits related to systems you’re not actively integrating with feels like a wrong thing to do in almost any situation.

In my case too, you'd only need to opt out of AutoreleaseSafe when you're doing stuff with Objective-C (or perhaps Swift, but they're close enough) autorelease pools.

I'll add that if a library wanted to make sure to opt out of all auto traits (e.g. if it implemented some sort of dynamic type), even for generic structs, they could do:

trait HelperTrait {}

struct MyTypeThatOptsOutOfEverything<T> {
    inner: T,
    p: PhantomData<dyn HelperTrait>,
}

Of course this is yet another SemVer footgun to be aware of, but fortunately we have tools for that now.

@GoldsteinE
Copy link
Contributor

Oops, I mixed up “opting out” and “opting in” in my original message. Either way, the point stands: I don’t see a reason to opt out of arbitrary auto traits unless you’re doing something that can affect this particular system. Author of the auto trait guarantees that it’s safe to impl it if all the fields impl it.

@steffahn
Copy link
Member

For Ungil, I’ve found the issue

which is that an auto-trait is unable to restrict access to certain types even if those types always feature a non-'static lifetime as provided by to a higher-kinded for<'a> FnOnce(SomeType<'a>)-style callback.

I’m relatively certain that the AutoreleaseSafe example suffers from the same issue.

Of course one approach would be to declare scoped-tls unsound on the (arguably quite thin) grounds that the standard library only sets precedent that 'static data is allowed to be shared via thread-local storage. Assuming we don’t declare scoped-tls unsound, an auto traits feature seems entirely unable to be a solution for sound API in either of the API cases for AutoreleaseSafe nor Ungil.

@GoldsteinE
Copy link
Contributor

Oh, that’s really sad. If scoped-tls is valid (and I don’t see a reason for it not to be), then auto trait Ungil can only make pyo3 less-obviously-unsound instead of sound. IMHO that’s still a valid reason for it to be an auto trait, but it’s really unfortunate that it doesn’t just make it clean and sound.

@madsmtm
Copy link
Contributor

madsmtm commented Dec 11, 2023

Hmm, I don't think that's a problem for objc2's AutoreleaseSafe, since the only type it's used on is AutoreleasePool<'pool>, whose only purpose is to carry a lifetime (it's a ZST that doesn't actually carry any data), which it seems like scoped-tls-hkt manages to properly bind to the execution of the closure.

So even though we can access the pool, we can't actually do anything bad with it, since it's only the lifetime that we care about, and that is still correctly bounded. See the following example for details.

```cargo
[dependencies]
# requires `auto_traits` feature
objc2 = { version = "0.5", features = ["unstable-autoreleasesafe"] }
scoped-tls-hkt = "0.1"
```

use objc2::rc::{autoreleasepool, AutoreleasePool, Id};
use objc2::runtime::NSObject;
use scoped_tls_hkt::scoped_thread_local;

fn main() {
    autoreleasepool(|pool1| {
        scoped_thread_local!(static POOL: for<'pool> AutoreleasePool<'pool>);

        POOL.set(pool1, || {
            let obj = autoreleasepool(|_pool2| {
                POOL.with(|pool1| {
                    Id::autorelease(NSObject::new(), pool1)
                })
            });

            // If we manage to get here, then that's unsound, since the object
            // would have been released in the second autorelease pool.

            println!("{obj:?}");
        });
    });
}

If you manage to produce a similar example that does compile, then I'd love to know!

@steffahn
Copy link
Member

@madsmtm Here we go:

use std::cell::Cell;

use objc2::rc::{autoreleasepool, AutoreleasePool, Id};
use objc2::runtime::NSObject;
use scoped_tls_hkt::scoped_thread_local;

struct PoolHolder<'p> {
    pool1: AutoreleasePool<'p>,
    obj_cell: &'p Cell<Option<&'p NSObject>>,
}
trait IPoolHolder {
    fn autorelease(&self, id: Id<NSObject>);
}
impl<'pool> IPoolHolder for PoolHolder<'pool> {
    fn autorelease(&self, id: Id<NSObject>) {
        self.obj_cell.set(Some(Id::autorelease(id, self.pool1)));
    }
}

fn main() {
    autoreleasepool(|pool1| {
        scoped_thread_local!(static POOL_HOLDER: for<'p> &'p dyn IPoolHolder);

        let obj_cell = &Cell::new(None);

        POOL_HOLDER.set(&PoolHolder { pool1, obj_cell }, || {
            autoreleasepool(|_pool2| {
                POOL_HOLDER.with(|pool1_holder| pool1_holder.autorelease(NSObject::new()))
            });

            let obj: &NSObject = obj_cell.get().unwrap();

            // If we manage to get here, then that's unsound, since the object
            // would have been released in the second autorelease pool.

            println!("{obj:?}");
        });
    });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system A-typesystem Area: The type system B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-auto_traits `#![feature(auto_traits)]` P-medium Medium priority S-tracking-perma-unstable Status: The feature will stay unstable indefinitely. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests