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

Permit uninhabited enums to cast into ints #76199

Merged
merged 2 commits into from
Oct 17, 2020

Conversation

Mark-Simulacrum
Copy link
Member

This essentially reverts part of #6204; it is unclear why that commit was introduced, and I suspect no one remembers.

The changed code was only called from casting checks and appears to not affect any callers of that code (other than permitting this one case).

Fixes #75647.

@Mark-Simulacrum Mark-Simulacrum added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 1, 2020
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(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 Sep 1, 2020
@Mark-Simulacrum
Copy link
Member Author

@rust-lang/lang, could someone kick off an fcp merge here?

This permits uninhabited enums to be as cast to integral types. This code is UB to execute due to the presence of the uninhabited value, so we do not need to specify the behavior of such a cast. The primary known use is for macros to not need to special-case this, though I believe we do not currently know of any macros which want this (likely due to no searching having been done).

@Mark-Simulacrum Mark-Simulacrum added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 1, 2020
@RalfJung
Copy link
Member

RalfJung commented Sep 1, 2020

Out of curiosity, what does such a cast compile to?

I'll certainly have to add a Miri test that actually reaches such a cast just to make sure it does not explode in horrible ways.^^

@joshtriplett
Copy link
Member

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Sep 1, 2020

Team member @joshtriplett has proposed to merge 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-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 1, 2020
@Mark-Simulacrum
Copy link
Member Author

Pushed up a commit fixing --emit=mir with this PR.

pub enum Void {}
#[no_mangle]
pub fn bar(v: Void) -> usize {
    v as usize
}

MIR:

fn bar(_1: Void) -> usize {
    debug v => _1;                       // in scope 0 at t.rs:3:12: 3:13
    let mut _0: usize;                   // return place in scope 0 at t.rs:3:24: 3:29
    let mut _2: Void;                    // in scope 0 at t.rs:4:5: 4:6

    bb0: {
        StorageLive(_2);                 // scope 0 at t.rs:4:5: 4:6
        _2 = move _1;                    // scope 0 at t.rs:4:5: 4:6
        _0 = move _2 as usize (Misc);    // scope 0 at t.rs:4:5: 4:15
        StorageDead(_2);                 // scope 0 at t.rs:4:14: 4:15
        return;                          // scope 0 at t.rs:5:2: 5:2
    }
}

LLVM IR:

define i64 @bar() unnamed_addr #0 {
start:
  ret i64 undef
}

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Miri changes LGTM.
I cannot comment on the other effects is_payloadfree might have.

@nikomatsakis
Copy link
Contributor

@rfcbot fcp reviewed

@rfcbot
Copy link

rfcbot commented Sep 14, 2020

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

@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 Sep 14, 2020
@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 Sep 24, 2020
@rfcbot
Copy link

rfcbot commented Sep 24, 2020

The final comment period, with a disposition to merge, 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.

The RFC will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Sep 24, 2020
@pickfire
Copy link
Contributor

Should we add a test based on the issue?

@Mark-Simulacrum
Copy link
Member Author

The PR already updates a pre-existing test; did you have something else in mind?

@pickfire
Copy link
Contributor

Test for #75647?

@jplatte
Copy link
Contributor

jplatte commented Sep 30, 2020

That's exactly what the uninhabited-enum-cast test is.

@spastorino spastorino removed the to-announce Announce this issue on triage meeting label Oct 1, 2020
@ctrl-z-9000-times
Copy link

The primary known use is for macros to not need to special-case this, though I believe we do not currently know of any macros which want this (likely due to no searching having been done).

Can confirm, this is an annoying corner case for my macro which generates custom enums.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 16, 2020

📌 Commit 990a395 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 Oct 16, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 17, 2020
…tsakis

Permit uninhabited enums to cast into ints

This essentially reverts part of rust-lang#6204; it is unclear why that [commit](rust-lang@c0f587d) was introduced, and I suspect no one remembers.

The changed code was only called from casting checks and appears to not affect any callers of that code (other than permitting this one case).

Fixes rust-lang#75647.
This was referenced Oct 17, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 17, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#76199 (Permit uninhabited enums to cast into ints)
 - rust-lang#77751 (liballoc: VecDeque: Add binary search functions)
 - rust-lang#77785 (Remove compiler-synthesized reexports when documenting)
 - rust-lang#77932 (BTreeMap: improve gdb introspection of BTreeMap with ZST keys or values)
 - rust-lang#77961 (Set .llvmbc and .llvmcmd sections as allocatable)
 - rust-lang#77985 (llvm: backport SystemZ fix for AGR clobbers)

Failed merges:

r? `@ghost`
@bors
Copy link
Contributor

bors commented Oct 17, 2020

⌛ Testing commit 990a395 with merge 3f50dea...

@bors bors merged commit 496e2fe into rust-lang:master Oct 17, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 17, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Feb 14, 2021
Pkgsrc changes:
 * Adjust patches, convert tabs to spaces so that tests pass.
 * Remove patches which are no longer needed (upstream changed)
 * Minor adjustments for SunOS, e.g. disable stack probes.
 * Adjust cargo checksum patching accordingly.
 * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway...

Upstream changes:

Version 1.49.0 (2020-12-31)
============================

Language
-----------------------

- [Unions can now implement `Drop`, and you can now have a field in a union
  with `ManuallyDrop<T>`.][77547]
- [You can now cast uninhabited enums to integers.][76199]
- [You can now bind by reference and by move in patterns.][76119] This
  allows you to selectively borrow individual components of a type. E.g.
  ```rust
  #[derive(Debug)]
  struct Person {
      name: String,
      age: u8,
  }

  let person = Person {
      name: String::from("Alice"),
      age: 20,
  };

  // `name` is moved out of person, but `age` is referenced.
  let Person { name, ref age } = person;
  println!("{} {}", name, age);
  ```

Compiler
-----------------------

- [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228]
- [Added tier 2 support for `aarch64-apple-darwin`.][75991]
- [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914]
- [Added tier 3 support for `mipsel-unknown-none`.][78676]
- [Raised the minimum supported LLVM version to LLVM 9.][78848]
- [Output from threads spawned in tests is now captured.][78227]
- [Change os and vendor values to "none" and "unknown" for some targets][78951]

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

Libraries
-----------------------

- [`RangeInclusive` now checks for exhaustion when calling `contains`
  and indexing.][78109]
- [`ToString::to_string` now no longer shrinks the internal buffer
  in the default implementation.][77997]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
  arrays of any length.][74989]

Stabilized APIs
---------------

- [`slice::select_nth_unstable`]
- [`slice::select_nth_unstable_by`]
- [`slice::select_nth_unstable_by_key`]

The following previously stable methods are now `const`.

- [`Poll::is_ready`]
- [`Poll::is_pending`]

Cargo
-----------------------
- [Building a crate with `cargo-package` should now be independently
  reproducible.][cargo/8864]
- [`cargo-tree` now marks proc-macro crates.][cargo/8765]
- [Added `CARGO_PRIMARY_PACKAGE` build-time environment
  variable.]  [cargo/8758] This variable will be set if the crate
  being built is one the user selected to build, either with `-p`
  or through defaults.
- [You can now use glob patterns when specifying packages &
  targets.][cargo/8752]


Compatibility Notes
-------------------
- [Demoted `i686-unknown-freebsd` from host tier 2 to target tier
  2 support.][78746]
- [Macros that end with a semi-colon are now treated as statements
  even if they expand to nothing.][78376]
- [Rustc will now check for the validity of some built-in attributes
  on enum variants.][77015] Previously such invalid or unused
  attributes could be ignored.
- Leading whitespace is stripped more uniformly in documentation
  comments, which may change behavior. You read [this post about
  the changes][rustdoc-ws-post] for more details.
- [Trait bounds are no longer inferred for associated types.][79904]

Internal Only
-------------
These changes provide no direct user facing benefits, but represent
significant improvements to the internals and overall performance
of rustc and related tools.

- [rustc's internal crates are now compiled using the `initial-exec` Thread
  Local Storage model.][78201]
- [Calculate visibilities once in resolve.][78077]
- [Added `system` to the `llvm-libunwind` bootstrap config option.][77703]
- [Added `--color` for configuring terminal color support to bootstrap.][79004]


[75991]: rust-lang/rust#75991
[78951]: rust-lang/rust#78951
[78848]: rust-lang/rust#78848
[78746]: rust-lang/rust#78746
[78376]: rust-lang/rust#78376
[78228]: rust-lang/rust#78228
[78227]: rust-lang/rust#78227
[78201]: rust-lang/rust#78201
[78109]: rust-lang/rust#78109
[78077]: rust-lang/rust#78077
[77997]: rust-lang/rust#77997
[77703]: rust-lang/rust#77703
[77547]: rust-lang/rust#77547
[77015]: rust-lang/rust#77015
[76199]: rust-lang/rust#76199
[76119]: rust-lang/rust#76119
[75914]: rust-lang/rust#75914
[74989]: rust-lang/rust#74989
[79004]: rust-lang/rust#79004
[78676]: rust-lang/rust#78676
[79904]: rust-lang/rust#79904
[cargo/8864]: rust-lang/cargo#8864
[cargo/8765]: rust-lang/cargo#8765
[cargo/8758]: rust-lang/cargo#8758
[cargo/8752]: rust-lang/cargo#8752
[`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable
[`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by
[`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key
[`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html
[`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready
[`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending
[rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc
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. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't as cast Void to usize