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

Remove static bound from type_id #1849

Merged
merged 5 commits into from
May 10, 2017
Merged

Remove static bound from type_id #1849

merged 5 commits into from
May 10, 2017

Conversation

KodrAus
Copy link
Contributor

@KodrAus KodrAus commented Jan 8, 2017

@sfackler sfackler added T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Jan 10, 2017
@burdges
Copy link

burdges commented Jan 12, 2017

One could not do any sensible reflection on lifetimes anyways, right?

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 12, 2017

That's true of the current state of the world. type_id has a 'static bound so it doesn't have to commit to considering lifetimes or not. This is more reflection on the type, which lives for some lifetime, rather than reflection on the lifetime itself.

What this RFC would let you do is get a type id for whatever you can stick in a Box<T>, whether that's T: 'static or the more general T: 'a. Then you can safely store and fetch data for arbitrary types in a type map that can be bound by some non-static lifetime.

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

How can you fetch data back? I can store something that points to the stack and then retrieve it with 'static lifetimes instead.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 12, 2017

The motivating example would work like this:

  • Define a marker trait like NonStaticAny, and implement for all T
  • Define your own NonStaticTypeId, that's basically the same as the one in std, minus the lifetime bound.
  • Define a type map like HashMap<NonStaticType Id, Box<NonStaticAny + 'a>>
  • You can then safely coerce an entry keyed by a type id of T into a *const T. And since you have a lifetime 'a that you know the data in the map lives for you can safely coerce the *const T into a &'a T.

I have an implementation of this in a sandbox repo. (Note it's just can we make this work? quality).

This is a fairly specific motivating usecase, but this RFC is an important baby step towards expanding the scope of reflection traits. It'll let us start to experiment with building APIs outside of the standard library and understand the landscape better.

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

No, you can't, I can put &'a T in and request &'static T and you can't tell the difference.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 12, 2017

Note you can probably do this with less unsafe code than my example, because it returns data with a lifetime that isn't tied to the borrow of the map. It's probably not worth focussing on the exact semantics of that though.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 12, 2017

@eddyb Ah I see what you mean! Yup that's a problem, I'll take that design back to the drawing board and see what I can do.

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

There is absolutely no way to use this with Any. If you try, I will break it, and I fear it will end up misused.
You need HKT/ATC, so you can have a T<'a>, require T<'static>: 'static and be invariant over 'a.
If you do any less, I can show you how it's broken. Oh and the best part? You don't need this RFC!
Why? Because for<'a> fn(T<'a>) should work with TypedId already.
Even better: you can encode ATC today, via <T as Trait<'a>>::Result, so you can do this right now.

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

To be clear: you can't use the TypeId of T<'static>, it has to be something with for<'a> ... T<'a>, otherwise I can store T<'a> = (&'a A, &'static B) and get U<'a> = (&'static A, &'a B) and they're identical if you substitute 'a with 'static. for<'a> is as close as you'll get to ecoding lifetime parametrism.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 12, 2017

Hmm I see your point. The idea was to treat the type id as a unique id for the type at runtime, and rely on binding pointers to that data to concrete lifetimes as soon as possible. It's obviously much more subtle than I thought though.

@glaebhoerl
Copy link
Contributor

I don't know about type_id itself, but I think it would be fairly straightforward to generalize Any by parameterizing it over a lifetime, with a 'static default for BC -- trait Any<'a = 'static>: 'a. This doesn't let you dynamically distinguish between different lifetimes, it just punts on the question of lifetimes entirely, so that you can at least cast between two types when both of them satisfy the same one (rather than when both are 'static).

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

What do you mean "satisfy"? How do you prevent me changing a lifetime from the upcast to the downcast?

@glaebhoerl
Copy link
Contributor

Instead of

impl Any + 'static {
    fn downcast_ref<T>(&self) -> Option<&T> where T: Any;
}

as we have currently, we would have:

impl<'a> Any<'a> + 'a {
    fn downcast_ref<T>(&self) -> Option<&T> where T: Any<'a> + 'a;
}

Here both the input (the Any) and the output (T) need to be valid for the same lifetime 'a, determined by the caller.

(I think those + 'a parts are redundant, because trait Any<'a>: 'a already requires that, but that's true currently with 'static as well, yet the impl is written impl Any + 'static, which I'm guessing may be due to a typechecker deficiency where it can't figure it out?)

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

"valid for the same lifetime" doesn't mean much. How do you prevent a 'b (larger than 'a) from turning into 'static without encoding the difference in TypeId somehow?

@glaebhoerl
Copy link
Contributor

Hm. We'd need to express that T must live for at most 'a, and we can only express at least?

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

Again, "valid for some lifetime" doesn't mean anything when you don't know the actual type.
You probably want "ensure all lifetimes are exactly this one" which doesn't have anything to do with validity or outliving of the entire type.

Lile I said, what you want is a lifetime -> type lambda you can get a TypeId from. Then you can inject whatever lifetime you want (I think Any<'a> would be invariant over 'a so at least that could work).

@strega-nil
Copy link

@eddyb in any case, this is the intrinsic; some (such as @sdleffler) would like to at least be able to play with it.

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

I'd like to see an example usecase that is not unsound (also, can they not use an HRTB encoding?).

@sdleffler
Copy link

@eddyb I don't know if you'd call it "sound", but I've been using TypeIds to encode existential versions of phantom-typed values. Since I don't actually hold the value any longer than its lifetime - I just make use of the TypeId - nothing's going to escape a scope, since the lifetime is "erased" when the type gets TypeId'd. A lack of lifetime restrictions would be quite useful for that, as it would remove the requirement for the user to touch the Keyed trait (I call it AsStatic in my code.) It's especially useful since I want to use types which are allocated with arenas as the phantom types, and those arena-allocated types contain references to other instances of the same type which live in the arena, and have the arena's lifetime. I can safely ensure lifetime correctness elsewhere in the program, and the phantom type still does its job.

On the other hand, I can keep AsStatic and write my own custom derive for it, but I'd rather not if there's a cleaner solution (and I think this one is.)

@eddyb
Copy link
Member

eddyb commented Jan 12, 2017

@sdleffler I am now convinced there is a valid usecase, but we have to put a lot of warnings, because it's unusable for actually having a lifetime-parametrized Any, which most people want.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 13, 2017

My usecase, a scoped IoC container, is a bit of a bust since it relied on the lifetime of the container borrow being shorter than the lifetime of the reference it returned. I thought I had the scope of that reference nailed down but, as @eddyb has shown, you can't do that. So it's unsound as it stands. I'll just go back to using ref counting for that.

@sdleffler I think you have a better case, I'll update the RFC to focus on the experiment/build with tools outside std, which is what you seem to be doing with bounded-any, rather than peddling my now-defunct motivation. Anything you can contribute to move it forward would be much appreciated!

@sdleffler
Copy link

@KodrAus bounded-any isn't actually the "real use case" I have to put forward here; the real use case I have looks something like this:

#[derive(Hash, PartialEq, Eq)]
pub struct Name<T>(Binding, PhantomData<T>);


#[derive(Hash, PartialEq, Eq)]
pub struct AnyName(Binding, TypeId);


#[derive(Clone, Hash, PartialEq, Eq)]
pub enum Binding {
    Free(String, u64),
    Bound(u64, u64),
}

This is a locally nameless variable encoding for doing substitution in, say, a lambda calculus. The funny thing about this example is that lifetimes are actually totally irrelevant, and this works fine for me; I use phantom types here to ensure that I am correctly substituting say, an Expr for an Expr (given a Name<Expr> to substitute with.) I also need to be able to abstract over what things are substituted for names so I can do things like collect the set of all free variables in a term. In this particular use case, any lifetimes are actually just artifacts of the arenas I'll be using to store these objects. In order to downcast an AnyName to a Name<T>, the T has to be supplied explicitly, and as such any lifetimes in it are valid in that scope; so even if the AnyName was created from what was originally a Name<Expr<'a>>, I can downcast the AnyName into a Name<Expr<'b>>, and everything will work safely, because 'b came from a context that it's necessarily valid in, and I can now substitute an Expr<'b> for a Name<Expr<'b>> just fine.

That said I'll be coming back to bounded-any sporadically, but I just wanted to clarify it's not my most pressing concern for this feature, especially since it can't be used as a trait object like Any (at the moment I have things separated out into BoundedAnyRef<'a> and other things, which is a mess.)

@KodrAus
Copy link
Contributor Author

KodrAus commented Jan 27, 2017

@sdleffler @eddyb I've replaced the motivation. It's a bit more hand-wavy now because I didn't want to describe the exact details of @sdleffler's usecase, but I can update it if you think that should be in there.

What do you think of the new intrinsic idea, that's specifically stripping concrete lifetimes? I'm not sure what the maintenance burden would be for forking the type id if the main implementation was updated. So it may be a silly idea.

@eddyb
Copy link
Member

eddyb commented Jan 27, 2017

FWIW the current intrinsic strips lifetimes (we can't actually preserve them to that point) and if you copy the libcore definition but remove 'static, it works fine.

@burdges
Copy link

burdges commented Mar 14, 2017

Just curious if I understand the issue with Any being 'static : You could deal with basic data structures that do not contain non-'static references, but if you contain a non-'static reference then your structure has a lifetime parameter that cannot be seen by simple type reflection, and thus cannot be preserved. To do reflection correctly, you might restrict Any to particular "lifetime kind", but doing so means you cannot allow any type parameters, as they might hide additional lifetimes. Is that about right?

@KodrAus
Copy link
Contributor Author

KodrAus commented Mar 14, 2017

@burdges Yeh, I think a great example @eddyb showed before is a tuple of (A: 'a, B: 'static). We assume 'a is the shorter lifetime, so this tuple is 'a. The problem is that has the same type id as (A: 'static, B: 'a). The tuple is still 'a, but now we've extended the lifetime of A beyond what it's actually valid for. If you're relying on that type id to encode lifetimes then you're going to end up supporting unsound behaviour, because of cases like that a simple scheme can't catch.

@burdges
Copy link

burdges commented Mar 14, 2017

I see. I hadn't even considered what happens if you get some 'static lifetimes in there. There is seemingly no reasonable notion of "lifetime kind" that does this, although if you've a specific collection of types then maybe you can make it work, albeit very carefully.

@eddyb
Copy link
Member

eddyb commented Mar 14, 2017

I did specify a scheme that should be sound though, by using ATC (or a trait with a lifetime parameter).
Ideally you'd write a type lambda but this shouldn't be much worse (and you can wrap common cases).

@pnkfelix
Copy link
Member

Hmm, this caveat from @eddyb:

As this is only proposing an unstable-only relaxation (of core::intrinsics::type_id) that can already be obtained by re-declaring the intrinsic (as we're currently not checking the 'static bound), I'm suggesting we move ahead and accept it, separate from any other endeavors to expand TypedId or Any.

is ... important to me. I would not be comfortable accepting this RFC if it were not for the above conditions (the most important one being that accessing this relies on an unstable feature anyway and would presumably remain behind that wall)...

I am debating about whether the RFC itself is sufficiently clear about how limited its scope is ... but I guess we aren't ever planning to stabilize compiler intrinsics...?

@pnkfelix
Copy link
Member

@rfcbot reviewed

@nagisa
Copy link
Member

nagisa commented Mar 15, 2017

I guess we aren't ever planning to stabilize compiler intrinsics

That’s right. Compiler intrinsics are permanently unstable. The caveat is that somebody proposing a stable wrapper later on might not know about this RFC at the time. That’s the most likely scenario to get a stable access to this intrinsic without 'static lifetime requirement.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 15, 2017

Compiler intrinsics are permanently unstable.

I do not agree with this at all. I think compiler intrinsics happen to all be unstable right now, but I expect we will stabilize some of them sooner or later.

@nikomatsakis
Copy link
Contributor

I do not agree with this at all. I think compiler intrinsics happen to all be unstable right now, but I expect we will stabilize some of them sooner or later.

Sorry, this was curt. To expand: I don't see any reason to say that "for ever and ever" we will never stabilize an intrinsic. For that matter, I view transmute as effectively an intrinsic and it is stable. I agree that the current mechanism of declaring intrinsics is not very good (and I keep meaning to propose to remove it altogether, in favor of lang-items) but I think we will definitely stabilize (and indeed have already done) "special items".

@KodrAus
Copy link
Contributor Author

KodrAus commented Mar 18, 2017

I would not be comfortable accepting this RFC if it were not for the above conditions (the most important one being that accessing this relies on an unstable feature anyway and would presumably remain behind that wall)...

@pnkfelix You're right this is relying on the fact that the intrinsic is unstable and users have to go looking for it, otherwise its potential edges would be a bigger problem. I think documentation on the intrinsic should be enough to communicate those edges in this case, but there is more we could do, like add a new intrinsic without the bound and mark it unsafe.

For type_id, I see the path to stabilisation being giving users stable alternatives to its use cases, like TypeId, and some future scheme that accounts for lifetimes. At least being able to opt-in to type_ids newly introduced edges directly we can play with some more of these use cases and have something concrete to work with for future RFCs.

@rfcbot
Copy link
Collaborator

rfcbot commented Mar 28, 2017

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

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Mar 28, 2017
@nrc nrc removed the I-nominated label Apr 6, 2017
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 7, 2017

The final comment period is now complete.

@KodrAus
Copy link
Contributor Author

KodrAus commented Apr 25, 2017

So now that the final comment period is complete what are the next steps for this RFC?

@nikomatsakis
Copy link
Contributor

The RFC will need to be merged. I just wrote up a description of the steps, here, as it happens...

@eddyb eddyb merged commit 6a9044b into rust-lang:master May 10, 2017
@eddyb
Copy link
Member

eddyb commented May 10, 2017

Huzzah! The @rust-lang/lang team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here: rust-lang/rust#41875

@mikeyhew
Copy link

mikeyhew commented Aug 2, 2018

@glaebhoerl

Hm. We'd need to express that T must live for at most 'a, and we can only express at least?

Have you considered opening an RFC for that? The syntax 'a: T would be a good place to start

@glaebhoerl
Copy link
Contributor

@mikeyhew I haven't, I don't even feel like I understand it.

@mikeyhew
Copy link

mikeyhew commented Aug 2, 2018

@glaebhoerl you're right, it is more complicated than I originally thought — if the type can't outlive 'a, does that mean you can't downcast to a type like i32?

@eddyb's suggestion of an Any<'a> trait looks more promising. I tried it out in a playground, and it seems to work. It uses the *const Self receiver for the type_id method so that it can be called on trait objects:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2015&gist=5fa7a0f75f6a2434d46e98de1542ee19

@Centril Centril added A-parametricity Proposals relating to parametricity. A-typesystem Type system related proposals & ideas A-reflection Proposals relating to compile time reflection. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parametricity Proposals relating to parametricity. A-reflection Proposals relating to compile time reflection. A-typesystem Type system related proposals & ideas final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.