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

ACP: Add const fn TypeId::matches for comparing type ids in consts #231

Closed
KodrAus opened this issue May 29, 2023 · 25 comments
Closed

ACP: Add const fn TypeId::matches for comparing type ids in consts #231

KodrAus opened this issue May 29, 2023 · 25 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@KodrAus
Copy link
Contributor

KodrAus commented May 29, 2023

Proposal

Problem statement

To provide an API for comparing TypeIds in const contexts that can be stabilized "soon" along with TypeId::of without depending on const in trait impls. This API should make stabilization of TypeId in const contexts uncontentious, but isn't interpreted as a commitment to actually do that stabilization.

Motivating examples or use cases

TypeIds can be used in const contexts as a limited form of specialization; they can be used to dispatch at compile-time based on the type of a value. As a real-world example, I have a library that allows capturing values using a standard trait, like fmt::Display, but specializes internally when that value is a primitive like a string or integer:

use std::{any::TypeId, fmt};

#[derive(Debug)]
pub enum Value<'a> {
    I32(i32),
    // Other variants for other primitives...
    Debug(&'a dyn fmt::Debug)
}

impl<'a> Value<'a> {
    pub fn capture_debug<T: fmt::Debug + 'static>(value: &'a T) -> Value<'a> {
        value_from_primitive(value).unwrap_or(Value::Debug(value))
    }
}

// NOTE: Can't currently be `const`; support was removed in https://github.com/rust-lang/rust/pull/103291
// `T` is unsized so `str` can be supported as well
const fn value_from_primitive<'a, T: ?Sized + 'static>(value: &'a T) -> Option<Value<'a>> {
    let id = TypeId::of::<T>();
    
    if id == TypeId::of::<i32>() {
        // Using `TypeId` instead of `Any::downcast` because `T` is unsized; so we can't convert into a `dyn Any`
        // without losing the `'static` bound
        // SAFETY: `T` has been asserted to be `i32`
        return Some(Value::I32(unsafe { *(value as *const T as *const i32) }))
    }

    // possibly many other branches...
    
    None
}

fn main() {
    // Prints `I32(42)`
    println!("{:?}", Value::capture_debug(&42i32));
    
    // Prints `Debug(42)`
    println!("{:?}", Value::capture_debug(&42u32));
}

This pattern is useful when working with dynamic data such as when templating or wiring up loosely-coupled state. Unfortunately, it doesn't currently work even on nightly because structural-matching of TypeIds has been removed (for perfectly valid reasons). The only thing you can do with a TypeId is compare it with other TypeIds, so without some const way to compare them, TypeId is currently useless in const contexts.

Solution sketch

This proposes TypeId::matches; an API for asserting two TypeIds match at compile-time that could be stabilized "soon" alongside TypeId::of to make TypeIds usable in const functions:

impl TypeId {
    /// Whether this type id is the same as `other`.
    ///
    /// If a `TypeId` matches another it means they were both instantiated from
    /// the same generic type `T`.
    ///
    /// This method is equivalent to equality, but can be used at compile-time.
    ///
    /// # Examples
    ///
    /// ```
    /// #![feature(const_type_id)]
    ///
    /// use std::any::TypeId;
    ///
    /// let typeof_string = TypeId::of::<String>();
    /// let typeof_bool = TypeId::of::<bool>();
    ///
    /// assert!(typeof_string.matches(typeof_string);
    /// assert!(!typeof_bool.matches(typeof_string);
    /// ```
    #[unstable(feature = "const_type_id", issue = "77125")]
    #[rustc_const_unstable(feature = "const_type_id", issue = "77125")]
    pub const fn matches(&self, other: TypeId) -> bool {
        self == other
    }
}

It relies on the standard library having some kind of support for const equality, without exposing what that support is. It decouples this pattern of specialization from const trait support or from full specialization.

Our example from before becomes:

const fn value_from_primitive<'a, T: ?Sized + 'static>(value: &'a T) -> Option<Value<'a>> {
    let id = TypeId::of::<T>();
    
    if id.matches(TypeId::of::<i32>()) {
        // Using `TypeId` instead of `Any::downcast` because `T` is unsized; so we can't convert into a `dyn Any`
        // without losing the `'static` bound
        // SAFETY: `T` has been asserted to be `i32`
        return Some(Value::I32(unsafe { *(value as *const T as *const i32) }))
    }

    // possibly many other branches...
    
    None
}

Alternatives

You could do this using plain-old ==, but that needs const trait support, which is still in its design stages. When equality does become possible in const contexts, this method is simply a semantic alternative to it that's also a good place to document what the implications of two TypeIds being equal or not are.

You could also use specialization, which isn't being actively pushed and has no clear path to stabilization.

You could use Any::is and Any::downcast_ref for this, but Any requires Sized + 'static (Sized for coercing to dyn Any and 'static from its trait bounds), which rules out unsized types like str.

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@KodrAus KodrAus added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels May 29, 2023
@KodrAus
Copy link
Contributor Author

KodrAus commented May 29, 2023

cc @eddyb who likes to scuttle all proposals relating to Any and TypeId 😁

@eddyb
Copy link
Member

eddyb commented May 29, 2023

who likes to scuttle all proposals relating to Any and TypeId

Uhm, as long as it's just const PartialEq, it doesn't matter (structural match was an issue because that's far stronger than ==, it amounts to "the compiler can implement Eq by itself without running user code", which rules out things like pointer address comparisons, even if we have const fn tools like guaranteed_eq which allow such checks at compile-time within a const PartialEq impl).

(frankly I think such a const_eq-style method is a waste of time, and stabilizing some const impls without stabilizing the syntax around them, might be a better plan, but I don't know enough about the status quo of those features to accurately speculate on possible timelines)

@KodrAus
Copy link
Contributor Author

KodrAus commented May 29, 2023

stabilizing some const impls without stabilizing the syntax around them, might be a better plan

I was working under the assumption that wasn't a possibility, but it may well be. I'm not really up-to-date on the status-quo of const trait impls myself.

@oli-obk
Copy link

oli-obk commented May 30, 2023

It would be possible, but the current implementation of the feature is very much in flux, so we'd like to avoid doing that in the near future. cc @fee1-dead

@fee1-dead
Copy link
Member

A min_const_trait_impl feature could allow defining impl const, and #[const_trait], and using trait methods, but that is only happening until we get everything with the new implementation right, and also requires an actual accepted RFC.

Another way would be to only allow desugared calls to PartialEq/PartialOrd under const_cmp. Stabilizing something like that would require lang team sign off on "we'd want == to work in const contexts for these std types, while not explicitly approving const traits as the underlying impl" , but I am not in a position to suggest that that would be something the lang team and wants to commit to.

@KodrAus
Copy link
Contributor Author

KodrAus commented May 30, 2023

Another way would be to only allow desugared calls to PartialEq/PartialOrd under const_cmp. Stabilizing something like that would require lang team sign off on "we'd want == to work in const contexts for these std types, while not explicitly approving const traits as the underlying impl"

From an end-user perspective that's at least already the case with many primitives so it doesn't seem like a big leap to allow some const equality to be exposed from std without also exposing that mechanism to end-users. It was also the case for const generics for a while so also isn't a surprising thing to do.

If the libs-api team would prefer not to add a special case const fn TypeId::matches for this and instead pursue getting some general way to stabilize const PartialEq/PartialOrd impls then I'd be happy to go do a proposal for them.

@scottmcm
Copy link
Member

Given that we have https://rust-lang.github.io/rfcs/2316-safe-unsafe-trait-methods.html and https://rust-lang.github.io/rfcs/3245-refined-impls.html, I think a feature-gated way to allow putting const fn in otherwise non-const impls and a feature-gated way to allow consuming those refined impls would be entirely with lang precedent.

(Actually exposing that to stable would, of course, need some sort of FCP or maybe more, but something like that in nightly seems entirely plausible to me, if it would be useful for people.)

@KodrAus
Copy link
Contributor Author

KodrAus commented May 31, 2023

Actually exposing that to stable would, of course, need some sort of FCP or maybe more

To be upfront, that's absolutely the goal of this proposal; to line things up so we could stabilize TypeId::of and some way to compare them in consts. These ACPs, as far as I know, aren't related to stabilization at all so understand that's not something we could or should commit to here, but I want to work towards making that possible, preferably without blocking on large active design work.

I want to work towards something we'd all be comfortable saying "we'd probably FCP that".

@scottmcm
Copy link
Member

I could certainly imagine FCPing the calling of eq/nes in const for certain types as a temporary measure, since it's already possible (for different reasons) on primitives. It reminds me of how we FCPed saying "we'll have const generic impls of these things (or some hack to make things work if necessary)" before we stabilized const generics in general.

@oli-obk
Copy link

oli-obk commented May 31, 2023

We could implement a hacky solution for non-generic impls methods today without relevant tech debt or complexity. We could allow adding rustc_const_stable and rustc_const_unstable to impl methods, allowing them to be used directly, without any of the usual other effects than an impl has (so generic functions still can't call methods on generic parameters). This effectively makes such impls work similarly to inherent const methods, except that various desugarings to the trait method will also work.

@KodrAus
Copy link
Contributor Author

KodrAus commented May 31, 2023

We could implement a hacky solution for non-generic impls methods today without relevant tech debt or complexity.

That sounds good to me. If we suddenly had a way to stabilize some methods on trait impls as const in std then I can imagine it would kick off a long tail of stabilize const PartialEq on X style requests. So if that's a direction we're happy to pursue I could write a proposal that suggested allowing it, and some guidelines for what traits we're comfortable stabilizing const methods on and any other restrictions or considerations for the types they're applied to.

Would that belong as a proposal in this repo here? Or is there a lang process I should follow?

@programmerjake
Copy link
Member

programmerjake commented May 31, 2023

well, a list of things i think need const PartialEq and const PartialOrd:

  • all integer types, char, bool, str, all nonzero types, Ordering, (), CStr, OsStr, PhantomData, Infallible, !

types that need ~const PartialEq/~const PartialOrd trait bounds:

  • arrays, slices, Option, Result, tuples, Wrapping (and similar)

@KodrAus
Copy link
Contributor Author

KodrAus commented Jun 1, 2023

I think those ~const impls would be disallowed from stable under this proposal because they have generics, right?

@programmerjake
Copy link
Member

programmerjake commented Jun 6, 2023

can we mark impl ~const PartialEq for TypeId unstably const now and switch to the hacky method later when that's implemented? I have code that could use it and i'd rather not have to DIY it by using a proc macro generating random numbers in traits and something like a Merkel hash tree...

@oli-obk
Copy link

oli-obk commented Jun 6, 2023

We removed all const trait impls and ~const bounds from libcore, because it was blocking replacing the implementation with something maintainable. So at present we cannot add that, even for nightly users.

@KodrAus
Copy link
Contributor Author

KodrAus commented Jun 7, 2023

We removed all const trait impls and ~const bounds from libcore, because it was blocking replacing the implementation with something maintainable. So at present we cannot add that, even for nightly users.

Ah, I see the machinery for const traits has been changing recently. Re-reading @oli-obk's original suggestion, I think it would just look something like this:

impl PartialEq for TypeId {
    #[rustc_const_stable(..)]
    fn eq(&self, other: &Self) -> bool { .. }
}

so that all we guarantee is that <TypeId as PartialEq>::eq can be called in CTFE. It doesn't say anything about PartialEq itself. Is that right?

@oli-obk
Copy link

oli-obk commented Jun 8, 2023

so that all we guarantee is that <TypeId as PartialEq>::eq can be called in CTFE. It doesn't say anything about PartialEq itself. Is that right?

Correct

@KodrAus
Copy link
Contributor Author

KodrAus commented Jun 13, 2023

Is this more of a lang proposal then and I should peddle it through their process somewhere? Or is it ok here? If so, I'll update the proposal to reflect the #[rustc_const_stable] business instead of suggesting new APIs on TypeId specifically.

@oli-obk
Copy link

oli-obk commented Jun 14, 2023

I believe this is still entirely in the purview of the libs team. We may do an FCP of both teams on the first PR that adds this though.

@m-ou-se
Copy link
Member

m-ou-se commented Jul 25, 2023

We discussed this just now in the libs-api meeting. We'd much prefer the regular == operator to work in const for this type (perhaps through some special attribute or other temporary rustc hack) than adding a method like this, as this method would immediately become obsolete when const trait impls become available.

@m-ou-se
Copy link
Member

m-ou-se commented Jul 9, 2024

A year later, it seems clear we're not close to having == work on TypeId in const context.

Given that, we're happy to see this as an unstable feature.

As for the name, we think matches might not be the best name as it doesn't clearly imply equality checking. Perhaps eq is a better name, or something const_eq if eq would cause conflicts.

Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature.

@m-ou-se m-ou-se added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Jul 9, 2024
@Amanieu Amanieu closed this as completed Jul 9, 2024
@RalfJung
Copy link
Member

To make this method not quite so pointless when const calls to == eventually happen, would it make sense to have a method like this?

    pub const fn matches<Other: 'static>(&self) -> bool {
        self == TypeId::of::<Other>()
    }

A year later, it seems clear we're not close to having == work on TypeId in const context.

I'm not aware that anyone tried to push for the solution suggested above. So unsurprisingly it did not happen.

@tgross35
Copy link

tgross35 commented Jul 10, 2024

To make this method not quite so pointless when const calls to == eventually happen, would it make sense to have a method like this?

    pub const fn matches<Other: 'static>(&self) -> bool {
        self == TypeId::of::<Other>()
    }

+1 that maybe this exact signature isn't the best method to keep around long term.

But I think maybe TypeId might not need to be in the signature at all - why not a standalone function like C++ has?

// Stealing the name `is_same::<T, U>()` from C++ `std::is_same<T, U>::value`

const fn is_same<A: 'static, B: 'static>() -> bool {
    // or intrinsic call
    TypeId::of::<A>().t == TypeId::of::<B>().t
}

It seems like the goal at comptime is probably not often actually the comparison of TypeIds, but rather to check if two generic types are the same, or compare a generic against a concrete type as in the example (happy to be corrected if this assumption is not accurate). Not needing to construct TypeIds is a cleaner API for these use cases, even after the point that const == for TypeId starts working.

Another benefit - not using TypeId in the signature means we keep the option open to make this an intrinsic and eventually stabilize it that way (using compiler machinery to check type equality), rather than being reliant on const TypeId::of (which seems to still have some hard questions to answer, some discussion).

@m-ou-se
Copy link
Member

m-ou-se commented Jul 10, 2024

That sounds reasonable to me. However, please open a new ACP if you want to propose a new API like that. Then we'll discuss it in the libs-api meeting. :)

@tgross35
Copy link

Thanks, I did so at #411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

10 participants