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 non_static_type_id #41875

Closed
3 tasks
eddyb opened this issue May 10, 2017 · 64 comments
Closed
3 tasks

Tracking issue for non_static_type_id #41875

eddyb opened this issue May 10, 2017 · 64 comments
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented May 10, 2017

This is a tracking issue for the RFC "1849" (rust-lang/rfcs#1849).

Steps:

@eddyb eddyb added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 10, 2017
@eddyb
Copy link
Member Author

eddyb commented May 10, 2017

Implementation instructions:

  • remove the 'static bound from the type_id intrinsic
  • add more cases to the typeid-intrinsic test:
    • in a function that has an 'a parameter and uses 'a in types
    • should check that non-'static lifetimes are allowed
    • also that they are ignored (e.g. &'a Foo and &'static Foo have the same type_id)

@eddyb eddyb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels May 10, 2017
@ghost
Copy link

ghost commented May 10, 2017

I'd like to participate on this :)

@eddyb
Copy link
Member Author

eddyb commented May 10, 2017

@z1mvader Great! Don't forget to put "r? @eddyb" in your PR when you open it.

@fcofdez
Copy link

fcofdez commented May 16, 2017

Is someone working on this issue? I would like to help with it.

@ghost
Copy link

ghost commented May 17, 2017

Hi, I'm currently working on it. I'm working on the tests

@kennytm
Copy link
Member

kennytm commented May 28, 2017

Could std::error::Error::{is, downcast_ref, downcast_mut} relax the 'static bound as well?

These currently require the 'static bound due to TypeId, and this makes it impossible to inspect the cause(), which just returns an Option<&Error> without 'static bound.

cc #35943

@eddyb
Copy link
Member Author

eddyb commented May 28, 2017

@kennytm Those sound like they do the same thing as Any, in which case relaxing them is unsound (see comments on rust-lang/rfcs#1849 for some more details).

@kennytm
Copy link
Member

kennytm commented May 29, 2017

@eddyb Thanks, I misread RFC. Sounds like doing this soundly will require &'a T and &'b T to encode to different TypeId, which is not possible because lifetime is not encoded in trans.

@eddyb
Copy link
Member Author

eddyb commented May 29, 2017

Not exactly. Lifetimes are a bit... "infinite" in their multitude.

What you want (like I mentioned in the RFC comments) is a scheme to specify where each of N lifetimes go (e.g. Foo<'a> = &'a T for one lifetime), enforce that there are no other lifetimes (e.g. Foo<'static>: 'static), encode that in a TypeId (e.g. for<'a> fn(Foo<'a>) - note that this is 'static and the for<'a> is preserved), then have one Any-like trait for every number of lifetimes (e.g. out of Box<Foo<'a>> you create Box<Any1<'a>>) and a way to downcast by providing the constructor (Foo *not *Foo<'something>).

This scheme requires HKT/ATC or even messier generic traits with HRTB. The main downside is the manual impls necessary without some kind of type lambda.

@Aceeri
Copy link

Aceeri commented Jun 16, 2017

@z1mvader Are you still working on this?

@ghost
Copy link

ghost commented Jun 16, 2017

i thought @kennytm was. I no longer have time sorry :(

@kennytm
Copy link
Member

kennytm commented Jun 16, 2017

@z1mvader o_O No I did not write any code on this, I was just making a comment in this thread 😅...

@Aceeri
Copy link

Aceeri commented Jun 28, 2017

I'll be working on this tomorrow, just so anyone who is watching this issue knows.

@Aceeri
Copy link

Aceeri commented Jul 2, 2017

Turns out this doesn't exactly help with my problem so I'm probably not going to be finishing my PR for this. Rustc also doesn't want to build properly on my machine, so I'd rather not waste too much time trying to fix it.

@dtolnay
Copy link
Member

dtolnay commented Jul 24, 2017

Here is a quick workaround for anyone waiting on this to be implemented.

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
pub struct TypeId {
    id: usize,
}

impl TypeId {
    pub fn of<T: ?Sized>() -> Self {
        TypeId {
            id: TypeId::of::<T> as usize,
        }
    }
}

Edit: Better:

use core::any::TypeId;
use core::marker::PhantomData;
use core::mem;

pub fn non_static_type_id<T: ?Sized>() -> TypeId {
    trait NonStaticAny {
        fn get_type_id(&self) -> TypeId where Self: 'static;
    }

    impl<T: ?Sized> NonStaticAny for PhantomData<T> {
        fn get_type_id(&self) -> TypeId where Self: 'static {
            TypeId::of::<T>()
        }
    }

    let phantom_data = PhantomData::<T>;
    NonStaticAny::get_type_id(unsafe {
        mem::transmute::<&dyn NonStaticAny, &(dyn NonStaticAny + 'static)>(&phantom_data)
    })
}

@eddyb
Copy link
Member Author

eddyb commented Jul 24, 2017

@dtolnay How... is that guaranteed to work, at all? Not to mention that having TypeId work, instead of just the intrinsic, suggests potential misuse. Do you want it for something Any-like? In that case, you're likely building something unsound.

@dtolnay
Copy link
Member

dtolnay commented Jul 24, 2017

It worked great when I tried it 🤷‍♂️. Here is the intrinsic version.

pub fn type_id<T: ?Sized>() -> u64 {
    type_id::<T> as usize as u64
}

@kennytm
Copy link
Member

kennytm commented Jul 24, 2017

@dtolnay I don't see how that's gonna work besides causing infinite recursion 😄

@eddyb
Copy link
Member Author

eddyb commented Jul 24, 2017

Note that it doesn't call, it casts a function pointer. But I expect mergefunc (-C opt-level=3 enables that optimization, if we still have it) might want to collapse all of those instances into one, and this clearly can't work cross-crate.

@dtolnay
Copy link
Member

dtolnay commented Jul 24, 2017

Seems to work with -C opt-level=3, and I don't need it cross-crate. 🎊

@eddyb
Copy link
Member Author

eddyb commented Jul 24, 2017

@dtolnay Still, there is nothing guaranteeing this won't be optimized away - if mergefunc is enabled right now, I'd even consider its failure to collapse those instances as a bug.
I consider it a bit irresponsible to suggest it to people who might write unsafe code off of it.

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 27, 2017
@Avi-D-coder
Copy link
Contributor

Even here - yes, it would be wrong to compare &f64 and &'static f64 for "type equality", but that's not a needed feature in this use case.

@bluss why is it wrong, to compare &f64 and &'static f64, according to the RFC they will always have the same TypeId, since lifetimes are erased.

@bluss
Copy link
Member

bluss commented May 2, 2020

If I understand correctly, it would be an ok verdict from the function itself, it's just dangerous if users act on it, for example, you might assume that if two types are equal (according to that comparison), then you can freely cast a raw pointer to one to the other type, and this breaks down with types that have lifetimes.

@Avi-D-coder
Copy link
Contributor

Avi-D-coder commented May 2, 2020

I think part of the problem with the conversation in this issue is we all mean slightly different things by type equality, wrong, safe and unsound.

If a cast is made as a result of the erroneous belief that TypeId equality implies lifetime equality, then I would say the cast is wrong/unsound not the TypeId comparison. And I might say type equality has been established, dispite the lack of lifetime equality.

While this feature clearly can be a foot-gun. The solution is to document that TypeId is blind to lifetimes rather than undoing the RFC. The ability to compare lifetime erased types is very useful to me, and the various workarounds are not general purpose. The confusion about and various unsound uses of this feature, could very well be a result of not reading the RFC, since the docs don't mention lifetime erasure at all.

If this feature were added to nightly with updated doc I don't think there would be very much confusion.

@joshtriplett
Copy link
Member

@rfcbot cancel

@rfcbot
Copy link

rfcbot commented May 2, 2020

@joshtriplett proposal cancelled.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels May 2, 2020
@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels May 2, 2020
@joshtriplett
Copy link
Member

@rfcbot close

@rfcbot
Copy link

rfcbot commented May 2, 2020

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels May 2, 2020
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 11, 2020
@rfcbot
Copy link

rfcbot commented May 11, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@nikomatsakis
Copy link
Contributor

It's probably worth adding a bit of context here. We discussed this in our recent @rust-lang/lang team meeting and the general feeling was that this feature just felt like it wasn't adding enough value, given the potential for confusion and misuse.

@rfcbot
Copy link

rfcbot commented May 21, 2020

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 21, 2020
@nikomatsakis
Copy link
Contributor

I don't believe this was ever implemented. I have pushed an update to the RFC indicating it was retracted. I'm going to close the tracking issue now.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 29, 2020
…komatsakis

Stabilize const_type_id feature

The tracking issue for `const_type_id` points to the ill-fated rust-lang#41875. So I'm re-energizing `TypeId` shenanigans by opening this one up to see if there's anything blocking us from stabilizing the constification of type ids.

Will wait for CI before pinging teams/groups.

-----

This PR stabilizes the `const_type_id` feature, which allows `TypeId::of` (and the underlying unstable intrinsic) to be called in constant contexts.

There are some [sanity tests](https://github.com/rust-lang/rust/blob/master/src/test/ui/consts/const-typeid-of-rpass.rs) that demonstrate its usage, but I’ve included some more below.

As a simple example, you could create a constant item that contains some type ids:

```rust
use std::any::TypeId;

const TYPE_IDS: [TypeId; 2] = [
    TypeId::of::<u32>(),
    TypeId::of::<i32>(),
];

assert_eq!(TypeId::of::<u32>(), TYPE_IDS[0]);
```

Type ids can also now appear in associated constants. You could create a trait that associates each type with its constant type id:

```rust
trait Any where Self: 'static {
    const TYPE_ID: TypeId = TypeId::of::<Self>();
}

impl<T: 'static> Any for T { }

assert_eq!(TypeId::of::<usize>(), usize::TYPE_ID);
```

`TypeId::of` is generic, which we saw above in the way the generic `Self` argument was used. This has some implications for const evaluation. It means we can make trait impls evaluate differently depending on information that wasn't directly passed through the trait system. This violates the _parametricity_ property, which requires all instances of a generic function to behave the same way with respect to its generic parameters. That's not unique to `TypeId::of`, other generic const functions based on compiler intrinsics like `mem::align_of` can also violate parametricity. In practice Rust doesn't really have type parametricity anyway since it monomorphizes generics into concrete functions, so violating it using type ids isn’t new.

As an example of how impls can behave differently, you could combine constant type ids with the `const_if_match` feature to dispatch calls based on the type id of the generic `Self`, rather than based on information about `Self` that was threaded through trait bounds. It's like a rough-and-ready form of specialization:

```rust
#![feature(const_if_match)]

trait Specialized where Self: 'static {
    // An associated constant that determines the function to call
    // at compile-time based on `TypeId::of::<Self>`.
    const CALL: fn(&Self) = {
        const USIZE: TypeId = TypeId::of::<usize>();

        match TypeId::of::<Self>() {
            // Use a closure for `usize` that transmutes the generic `Self` to
            // a concrete `usize` and dispatches to `Self::usize`.
            USIZE => |x| Self::usize(unsafe { &*(x as *const Self as *const usize) }),
            // For other types, dispatch to the generic `Self::default`.
            _ => Self::default,
        }
    };

    fn call(&self) {
        // Call the function we determined at compile-time
        (Self::CALL)(self)
    }

    fn default(x: &Self);
    fn usize(x: &usize);
}

// Implement our `Specialized` trait for any `Debug` type.
impl<T: fmt::Debug + 'static> Specialized for T {
    fn default(x: &Self) {
        println!("default: {:?}", x);
    }

    fn usize(x: &usize) {
        println!("usize: {:?}", x);
    }
}

// Will print "usize: 42"
Specialized::call(&42usize);

// Will print "default: ()"
Specialized::call(&());
```

Type ids have some edges that this stabilization exposes to more contexts. It's possible for type ids to collide (but this is a bug). Since they can change between compiler versions, it's never valid to cast a type id to its underlying value.
@dtolnay
Copy link
Member

dtolnay commented Jul 24, 2022

I put up a pre-RFC in https://internals.rust-lang.org/t/pre-rfc-non-footgun-non-static-typeid/17079 with a counterproposal that unblocks many of the use cases that wanted non-'static TypeId, but in a way that does not open up the risks that have been discussed above in this issue.

@eddyb
Copy link
Member Author

eddyb commented Jul 24, 2022

There was one usecase which came up early on IIRC, that wouldn't be solved by the "dynamic conservative : 'static check" shenanigans, which was something like TypeId::of::<Foo<'a>>() being used as a workaround TypeId::of::<FooTag>().

Nevertheless, I'm extremely wary of anything allowing actual lifetimes in there, compared to only doing the : 'static check in a different way (which "just" requires good API design & docs).

(If anyone is wondering what it takes to have an Any that soundly handles lifetimes, I sketched the kind of necessary design recently: sagebind/castaway#6 (comment) - if you're doing MyAny<'a> and it doesn't look at least half as complicated as that sketch, it's probably wrong and can be misused to transmute lifetimes away)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests