Turn `type_id` into a constant intrinsic by Badel2 · Pull Request #47892 · rust-lang/rust

Badel2 · 2018-01-30T22:47:55Z

The method get_type_id in Any is intended to support reflection. It's currently unstable in favor of using an associated constant instead. This PR makes the type_id intrinsic a constant intrinsic, the same as size_of and align_of, allowing TypeId::of to be a const fn, which will allow using an associated constant in Any.

rust-highfive · 2018-01-30T22:48:08Z

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

Badel2 · 2018-01-30T23:03:14Z

I basically copied @eddyb 's #42859, as it's my first time dealing with compiler instrinsics.

In miri/const_eval.rs there's an error called "NeedsRfc", which would suggest that adding intrinsics to const_eval needs some discussion, so I'm not sure if my PR would be allowed.

I added a few lines of code to an existing test because I don't know what exactly to test, so any suggestions welcome.

As for the Any trait, I tried adding a simple const TYPE_ID: TypeId = TypeId::of::<Self>(); but it didn't compile, I get an error (E0038) that says "the trait cannot contain associated consts" and I'm not sure why. Since that is the goal of this PR, I would like to make sure that adding an associated constant to Any is possible, and maybe do it in this PR.

eddyb · 2018-01-30T23:05:40Z

cc @oli-obk @durka This probably needs to indicate that it's not stably const fn.

oli-obk · 2018-01-31T06:05:52Z

Do the type IDs change between compilations?

According to the docs they'll change between releases and platforms. I guess this could be said of type_size, too, but that one is at least predictable in some manner.

Should definitely be unstable const fn until it can actually be added to Any

durka · 2018-01-31T06:15:11Z

As I understand they're basically keys to a hashmap, right? So completely unstable between compiler versions. Like enum discriminants but worse. However, that's already the case with TypeId::of.

…

On Wed, Jan 31, 2018 at 7:06 AM, Oliver Schneider ***@***.***> wrote: Do the type IDs change between compilations? According to the docs they'll change between releases and platforms. I guess this could be said of type_size, too, but that one is at least predictable in some manner. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#47892 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n9yn9Ua4qxNMpFhnbnGmm1cbuBPRks5tQALWgaJpZM4RzI_i> .

durka · 2018-01-31T06:16:39Z

`#[rustc_const_unstable(feature="const_type_id")]` is what @eddyb is asking for.

…

On Wed, Jan 31, 2018 at 7:14 AM, Alex Burka ***@***.***> wrote: As I understand they're basically keys to a hashmap, right? So completely unstable between compiler versions. Like enum discriminants but worse. However, that's already the case with TypeId::of. On Wed, Jan 31, 2018 at 7:06 AM, Oliver Schneider < ***@***.***> wrote: > Do the type IDs change between compilations? > > According to the docs they'll change between releases and platforms. I > guess this could be said of type_size, too, but that one is at least > predictable in some manner. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#47892 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAC3n9yn9Ua4qxNMpFhnbnGmm1cbuBPRks5tQALWgaJpZM4RzI_i> > . >

oli-obk · 2018-01-31T07:07:04Z

So completely unstable between compiler versions. Like enum discriminants but worse.

With miri we could make sure that no decisions are made on that value.

durka · 2018-01-31T08:01:38Z

Uh... define "decisions"? That sounds like a potentially really annoying restriction.

…

On Jan 31, 2018 08:07, "Oliver Schneider" ***@***.***> wrote: So completely unstable between compiler versions. Like enum discriminants but worse. With miri we could make sure that no decisions are made on that value. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#47892 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n_sP8od1eq9k7Xn1ldjMLrKI7Xe9ks5tQBEtgaJpZM4RzI_i> .

oli-obk · 2018-01-31T08:16:11Z

You can't use it as an index to arrays, you can't compare it to anything but other values derived from type_id, ...

eddyb · 2018-01-31T08:18:36Z

I'd rather not go crazy about it - it's your fault if you misuse this random (but deterministic within one compilation) number that the compiler gave you.

Badel2 · 2018-01-31T12:19:20Z

#[rustc_const_unstable(feature="const_type_id")]

Thanks @durka, but it was really hard to find documentation about it. I assume that when you add it to a stable function, and that function is called in a non-const context, it works the same as before? Because TypeId::of is already stable so it should keep working. Or maybe do I need to add rustc_const_unstable to the intrinsic itself? I have run rg rustc_const_unstable src/ and there are zero uses of this feature outside of tests, so I don't know what to do.

oli-obk · 2018-01-31T15:06:24Z

This should be in a separate file to ensure that TypeId::of without the feature enabled still works

I agree, this test is only temporal as I didn't know what to test. (It still works btw).

dtolnay · 2018-01-31T16:56:41Z

Seems reasonable to me, @oli-obk please merge when ready. Thanks!

@bors delegate=oli-obk

bors · 2018-01-31T16:56:42Z

✌️ @oli-obk can now approve this pull request

Badel2 · 2018-02-01T19:06:07Z

ping @oli-obk

oli-obk

Just a style thing, Lgtm otherwise

oli-obk · 2018-02-01T21:31:18Z

Ah, this is a good test that I forgot about. What I meant was an extra file that uses TypeId::of without the feature gate in a non const context.
....
And a quick search shows there are already files testing that. Don't mind me

oli-obk · 2018-02-01T21:37:29Z

Nit: remove this empty line

Add rustc_const_unstable attribute for `any::TypeId::of` Add test for `const fn TypeId::of`

dtolnay · 2018-02-04T17:20:01Z

@bors r=oli-obk

bors · 2018-02-04T17:20:02Z

📌 Commit 196fad0 has been approved by oli-obk

Turn `type_id` into a constant intrinsic rust-lang#27745 The method `get_type_id` in `Any` is intended to support reflection. It's currently unstable in favor of using an associated constant instead. This PR makes the `type_id` intrinsic a constant intrinsic, the same as `size_of` and `align_of`, allowing `TypeId::of` to be a `const fn`, which will allow using an associated constant in `Any`.

Rollup of 10 pull requests - Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892 - Failed merges:

rust-highfive assigned dtolnay Jan 30, 2018

kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 31, 2018

oli-obk reviewed Jan 31, 2018
View reviewed changes

Badel2 force-pushed the const_type_id_of branch 2 times, most recently from 2da171f to 80cb8b3Compare January 31, 2018 16:40

kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2018

oli-obk requested changes Feb 1, 2018
View reviewed changes

Turn type_id into a constant intrinsic
196fad0
Add rustc_const_unstable attribute for `any::TypeId::of` Add test for `const fn TypeId::of`

Badel2 force-pushed the const_type_id_of branch from 80cb8b3 to 196fad0Compare February 1, 2018 22:03

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 4, 2018

kennytm mentioned this pull request Feb 4, 2018
Rollup of 10 pull requests #47998
Merged

bors added a commit that referenced this pull request Feb 4, 2018
Auto merge of#47998- kennytm:rollup, r=kennytm
7dd3864
Rollup of 10 pull requests - Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892 - Failed merges:

bors added a commit that referenced this pull request Feb 4, 2018
Auto merge of#47998- kennytm:rollup, r=kennytm
e7e982a
Rollup of 10 pull requests - Successful merges: #47862, #47877, #47896, #47912, #47947, #47958, #47978, #47996, #47999, #47892 - Failed merges:

bors merged commit 196fad0 into rust-lang:masterFeb 5, 2018

KodrAus mentioned this pull request Sep 23, 2020
Tracking Issue for const fntype_id#77125
Closed
4 tasks

Uh oh!

Uh oh!

Conversation

Badel2 commented Jan 30, 2018

Uh oh!

rust-highfive commented Jan 30, 2018

Uh oh!

Badel2 commented Jan 30, 2018

Uh oh!

eddyb commented Jan 30, 2018

Uh oh!

oli-obk commented Jan 31, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

durka commented Jan 31, 2018 via email

Uh oh!

durka commented Jan 31, 2018 via email

Uh oh!

oli-obk commented Jan 31, 2018

Uh oh!

durka commented Jan 31, 2018 via email

Uh oh!

oli-obk commented Jan 31, 2018

Uh oh!

eddyb commented Jan 31, 2018

Uh oh!

Badel2 commented Jan 31, 2018

Uh oh!

oli-obkJan 31, 2018

Choose a reason for hiding this comment

Uh oh!

Badel2Jan 31, 2018

Choose a reason for hiding this comment

Uh oh!

dtolnay commented Jan 31, 2018

Uh oh!

bors commented Jan 31, 2018

Uh oh!

Badel2 commented Feb 1, 2018

Uh oh!

oli-obk left a comment

Choose a reason for hiding this comment

Uh oh!

oli-obkFeb 1, 2018

Choose a reason for hiding this comment

Uh oh!

oli-obkFeb 1, 2018

Choose a reason for hiding this comment

Uh oh!

dtolnay commented Feb 4, 2018

Uh oh!

bors commented Feb 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

oli-obk commented Jan 31, 2018•
edited
Loading