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

Stabilize const_type_id feature #72488

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Stabilize const_type_id feature #72488

merged 4 commits into from
Jul 29, 2020

Conversation

KodrAus
Copy link
Contributor

@KodrAus KodrAus commented May 23, 2020

The tracking issue for const_type_id points to the ill-fated #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 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:

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:

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:

#![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.

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2020
@joshtriplett
Copy link
Member

r? @ecstatic-morse

@KodrAus KodrAus added relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels May 23, 2020
@jonas-schievink jonas-schievink added this to the 1.45 milestone May 23, 2020
@ecstatic-morse
Copy link
Contributor

I don't see any hazards from this. The result of type_id is allowed to change across compiler versions, but I think that is acceptable for const fn.

cc @rust-lang/wg-const-eval

@RalfJung
Copy link
Member

Cc @rust-lang/lang because this is about const-stabilizing an intrinsic.

@Y0ba
Copy link

Y0ba commented May 24, 2020

What about collisions #10389? Currently this API is really dangerous to use and there is no way to detect collisions even without using dylibs. Not really related to const-stabilization but in current shape it's pretty useless both in const and in runtime.

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented May 24, 2020

In my opinion, the fact that type_id is poorly implemented doesn't have any bearing on whether or not it can be const. All code that uses Any is using type_id under the hood, so it's not "pretty useless". While I totally agree that the chance of a hash-collision is a serious problem, saying it's "really dangerous" is an overstatement for the vast majority of programs: The probability of a 64-bit hash collision in a program with 100,000 types is on the order of 1 in 10 billion.

@RalfJung

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@RalfJung

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@Y0ba
Copy link

Y0ba commented May 24, 2020

It''s possible to intentionally find type_id collisions.

Unfortunately it's possible to hit collisions unintentionally.

Why should that affect const-stabilization?

Because there will be more ways to hit it. That's what I wanted to say. I'd rather see that feature deprecated and switch Any trait to safer alternative.

@RalfJung

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@RalfJung

This comment has been minimized.

@RalfJung

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@RalfJung

This comment has been minimized.

@ecstatic-morse

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 27, 2020
@KodrAus
Copy link
Contributor Author

KodrAus commented Jul 27, 2020

Ah looks like I’ve got a test to fix up:


error[E0635]: unknown feature `const_type_id`
12379
  --> /checkout/src/test/ui/consts/issue-73976-monomorphic.rs:8:12
12380
   |
12381
LL | #![feature(const_type_id)]
12382
   |            ^^^^^^^^^^^^^
12383

12384
error: aborting due to previous error

@bors
Copy link
Contributor

bors commented Jul 28, 2020

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2020

📌 Commit 9d4818c has been approved by nikomatsakis

@bors bors 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 Jul 29, 2020
@bors
Copy link
Contributor

bors commented Jul 29, 2020

⌛ Testing commit 9d4818c with merge 6fd4c3f...

@bors
Copy link
Contributor

bors commented Jul 29, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: nikomatsakis
Pushing 6fd4c3f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 29, 2020
@bors bors merged commit 6fd4c3f into rust-lang:master Jul 29, 2020
@KodrAus KodrAus deleted the stabilize/const_type_id branch July 30, 2020 03:42
@jplatte
Copy link
Contributor

jplatte commented Aug 3, 2020

Stability attribute is 1.46, has to be updated to 1.47.

@eddyb
Copy link
Member

eddyb commented Aug 28, 2020

Just came across this, some of the people involved in this might be interested in #75923 (comment).

@scottmcm
Copy link
Member

scottmcm commented Sep 2, 2020

Interesting, eddyb. FWIW I'd be fine with reverting this on beta under a "new information came in that made us not confident in the promise any more" justification. Wouldn't be the first rolled-back stabilization...

@RalfJung
Copy link
Member

See this discussion for why stabilization was likely premature, and we should take it back before the release.

@KodrAus can you create the PR that reverts this, or should I?

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 24, 2020
revert const_type_id stabilization

This reverts rust-lang#72488, which is currently on beta and scheduled to stabilize in `1.47.0`, based on rust-lang#75923 (comment)

It turns out we might not be quite ready to stabilize `TypeId` in const contexts before having a chance to rework its internals. Since `TypeId` is a bit of an oddity we want to be careful about how those internals are currently being relied on while making changes. That will be easier to do without having to also consider compile-time contexts.

r? `@eddyb`
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 13, 2020
Pkgsrc changes:
 * Remove patches now integrated upstream, many related to SunOS / Illumos.
 * The LLVM fix for powerpc is also now integrated upstream.
 * Adapt those patches where the source has moved or parts are integrated.
 * The randomness patches no longer applies, and I could not find
   where those files went...
 * Provide a separate bootstrap for NetBSD/powerpc 9.0, since apparently
   the C++ ABI is different from 8.0.  Yes, this appears to be specific to
   the NetBSD powerpc ports.

Upstream changes:

Version 1.47.0 (2020-10-08)
==========================

Language
--------
- [Closures will now warn when not used.][74869]

Compiler
--------
- [Stabilized the `-C control-flow-guard` codegen option][73893], which enables
  [Control Flow Guard][1.47.0-cfg] for Windows platforms, and is ignored on
  other platforms.
- [Upgraded to LLVM 11.][73526]
- [Added tier 3\* support for the `thumbv4t-none-eabi` target.][74419]
- [Upgrade the FreeBSD toolchain to version 11.4][75204]
- [`RUST_BACKTRACE`'s output is now more compact.][75048]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`CStr` now implements `Index<RangeFrom<usize>>`.][74021]
- [Traits in `std`/`core` are now implemented for arrays of any length, not just
  those of length less than 33.][74060]
- [`ops::RangeFull` and `ops::Range` now implement Default.][73197]
- [`panic::Location` now implements `Copy`, `Clone`, `Eq`, `Hash`, `Ord`,
  `PartialEq`, and `PartialOrd`.][73583]

Stabilized APIs
---------------
- [`Ident::new_raw`]
- [`Range::is_empty`]
- [`RangeInclusive::is_empty`]
- [`Result::as_deref`]
- [`Result::as_deref_mut`]
- [`Vec::leak`]
- [`pointer::offset_from`]
- [`f32::TAU`]
- [`f64::TAU`]

The following previously stable APIs have now been made const.

- [The `new` method for all `NonZero` integers.][73858]
- [The `checked_add`,`checked_sub`,`checked_mul`,`checked_neg`, `checked_shl`,
  `checked_shr`, `saturating_add`, `saturating_sub`, and `saturating_mul`
  methods for all integers.][73858]
- [The `checked_abs`, `saturating_abs`, `saturating_neg`, and `signum`  for all
  signed integers.][73858]
- [The `is_ascii_alphabetic`, `is_ascii_uppercase`, `is_ascii_lowercase`,
  `is_ascii_alphanumeric`, `is_ascii_digit`, `is_ascii_hexdigit`,
  `is_ascii_punctuation`, `is_ascii_graphic`, `is_ascii_whitespace`, and
  `is_ascii_control` methods for `char` and `u8`.][73858]

Cargo
-----
- [`build-dependencies` are now built with opt-level 0 by default.][cargo/8500]
  You can override this by setting the following in your `Cargo.toml`.
  ```toml
  [profile.release.build-override]
  opt-level = 3
  ```
- [`cargo-help` will now display man pages for commands rather just the
  `--help` text.][cargo/8456]
- [`cargo-metadata` now emits a `test` field indicating if a target has
  tests enabled.][cargo/8478]
- [`workspace.default-members` now respects `workspace.exclude`.][cargo/8485]
- [`cargo-publish` will now use an alternative registry by default if it's the
  only registry specified in `package.publish`.][cargo/8571]

Misc
----
- [Added a help button beside Rustdoc's searchbar that explains rustdoc's
  type based search.][75366]
- [Added the Ayu theme to rustdoc.][71237]

Compatibility Notes
-------------------
- [Bumped the minimum supported Emscripten version to 1.39.20.][75716]
- [Fixed a regression parsing `{} && false` in tail expressions.][74650]
- [Added changes to how proc-macros are expanded in `macro_rules!` that should
  help to preserve more span information.][73084] These changes may cause
  compiliation errors if your macro was unhygenic or didn't correctly handle
  `Delimiter::None`.
- [Moved support for the CloudABI target to tier 3.][75568]
- [`linux-gnu` targets now require minimum kernel 2.6.32 and glibc 2.11.][74163]

Internal Only
--------
- [Improved default settings for bootstrapping in `x.py`.][73964]
  You can read details about this change in the ["Changes to `x.py`
  defaults"](https://blog.rust-lang.org/inside-rust/2020/08/30/changes-to-x-py-defaults.html)
  post on the Inside Rust blog.

- [Added the `rustc-docs` component.][75560] This allows you to install
  and read the documentation for the compiler internal APIs. (Currently only
  available for `x86_64-unknown-linux-gnu`.)

[1.47.0-cfg]: https://docs.microsoft.com/en-us/windows/win32/secbp/control-flow-guard
[76980]: rust-lang/rust#76980
[75048]: rust-lang/rust#75048
[74163]: rust-lang/rust#74163
[71237]: rust-lang/rust#71237
[74869]: rust-lang/rust#74869
[73858]: rust-lang/rust#73858
[75716]: rust-lang/rust#75716
[75908]: rust-lang/rust#75908
[75516]: rust-lang/rust#75516
[75560]: rust-lang/rust#75560
[75568]: rust-lang/rust#75568
[75366]: rust-lang/rust#75366
[75204]: rust-lang/rust#75204
[74650]: rust-lang/rust#74650
[74419]: rust-lang/rust#74419
[73964]: rust-lang/rust#73964
[74021]: rust-lang/rust#74021
[74060]: rust-lang/rust#74060
[73893]: rust-lang/rust#73893
[73526]: rust-lang/rust#73526
[73583]: rust-lang/rust#73583
[73084]: rust-lang/rust#73084
[73197]: rust-lang/rust#73197
[72488]: rust-lang/rust#72488
[cargo/8456]: rust-lang/cargo#8456
[cargo/8478]: rust-lang/cargo#8478
[cargo/8485]: rust-lang/cargo#8485
[cargo/8500]: rust-lang/cargo#8500
[cargo/8571]: rust-lang/cargo#8571
[`Ident::new_raw`]:  https://doc.rust-lang.org/nightly/proc_macro/struct.Ident.html#method.new_raw
[`Range::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.Range.html#method.is_empty
[`RangeInclusive::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.RangeInclusive.html#method.is_empty
[`Result::as_deref_mut`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref_mut
[`Result::as_deref`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref
[`TypeId::of`]: https://doc.rust-lang.org/nightly/std/any/struct.TypeId.html#method.of
[`Vec::leak`]: https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.leak
[`f32::TAU`]: https://doc.rust-lang.org/nightly/std/f32/consts/constant.TAU.html
[`f64::TAU`]: https://doc.rust-lang.org/nightly/std/f64/consts/constant.TAU.html
[`pointer::offset_from`]: https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.offset_from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.