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

Add unstable core::mem::variant_count intrinsic #73418

Merged
merged 3 commits into from
Jun 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1917,6 +1917,15 @@ extern "rust-intrinsic" {
#[rustc_const_unstable(feature = "const_discriminant", issue = "69821")]
pub fn discriminant_value<T>(v: &T) -> <T as DiscriminantKind>::Discriminant;

/// Returns the number of variants of the type `T` cast to a `usize`;
Copy link
Member

@Aaron1011 Aaron1011 Jun 16, 2020

Choose a reason for hiding this comment

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

Returning a usize is incompatible with #![feature(repr128)].

We don't currently expose a way for users to determine the discriminant type of an enum (see #70705). I think we have a few options here:

  1. Make this function return a u128. Assuming we never add u256/i256 to the language (both of which seem insane as an enum discriminant type), this would be compatible with enums with any number of variants.
  2. Stabilize the DiscriminantKind trait, and have this function return a value of type <T as DiscriminantKind>::Discrimininant>. This does not solve the problem of converting the value to an integer, but it avoids returning a u128 for enums which don't need it.
  3. Declare that enums with more than usize::MAX variants are unsupported (#[repr(u128)] enums with fewer variants are fine). We might want to consider changing this to u64, though I don't actually see any use-case for having an enum with more than 4 billion variants (let alone on a 32-bit platform).

Copy link
Member

Choose a reason for hiding this comment

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

On second thought, Rust does have tier-3 support for a 16-bit platform (AVR). While having u16::MAX variants seems ridiculous, it seems slightly more questionable to forbid that compared to u32::MAX (though again, I have no idea why anyone would do such a thing). Perhaps a u32 is the way to go here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seeing as the compiler uses an IdxVec to track declared variants we’re guaranteed to ICE if we have an enum with a number of variants exceeding usize::MAX anyway so I think they’re implicitly unsupported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making it explicit is no bad thing though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah - I completely forgot about cross compilation... u64 seems like the best bet then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok... Thinking about this some more, I think we stick with usize and add a note to say that if your enum has more than usize::MAX variants on the target platform, the return value is unspecified. The probability this ever happens is minuscule and we've covered our backs with the note.

Copy link
Member

Choose a reason for hiding this comment

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

Seeing as the compiler uses an IdxVec to track declared variants we’re guaranteed to ICE if we have an enum with a number of variants exceeding usize::MAX anyway so I think they’re implicitly unsupported.

The compiler could be running on a 64bit system but generate code for a 16bit system. So you cannot really deduce anything from the type the compiler is using here.

Copy link
Member

@nagisa nagisa Jun 25, 2020

Choose a reason for hiding this comment

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

I’m in favour of <T as DiscriminantKind>::Discrimininant> or u128 or u64, in that specific order. Having safe and stable APIs that may return unspecified or incorrect results without a very very convincing reason is questionable.

uses an IdxVec to track declared variants we’re guaranteed to ICE if we have an enum with a number of variants exceeding usize::MAX anyway

This is an implementation detail that can be changed in the future to adapt to changing needs. The return type of a stable API will get set in stone for however long Rust exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, you can’t use <T as DiscriminantKind>::Discriminant because if I have an enum with 256 variants the discriminant type will be u8 which can’t represent 256.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seeing as the compiler uses an IdxVec to track declared variants we’re guaranteed to ICE if we have an enum with a number of variants exceeding usize::MAX anyway so I think they’re implicitly unsupported.

I should have struck this out because it doesn’t apply anyway in the case of cross comp

doctorn marked this conversation as resolved.
Show resolved Hide resolved
/// if `T` has no variants, returns 0. Uninhabited variants will be counted.
///
/// The to-be-stabilized version of this intrinsic is
/// [`std::mem::variant_count`](../../std/mem/fn.variant_count.html)
#[rustc_const_unstable(feature = "variant_count", issue = "73662")]
#[cfg(not(bootstrap))]
pub fn variant_count<T>() -> usize;

/// Rust's "try catch" construct which invokes the function pointer `try_fn`
/// with the data pointer `data`.
///
Expand Down Expand Up @@ -1960,6 +1969,12 @@ extern "rust-intrinsic" {
pub fn ptr_guaranteed_ne<T>(ptr: *const T, other: *const T) -> bool;
}

#[rustc_const_unstable(feature = "variant_count", issue = "73662")]
#[cfg(bootstrap)]
pub const fn variant_count<T>() -> usize {
0
}

// Some functions are defined here because they accidentally got made
// available in this module on stable. See <https://github.com/rust-lang/rust/issues/15702>.
// (`transmute` also falls into this category, but it cannot be wrapped due to the
Expand Down
1 change: 1 addition & 0 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
#![feature(unsized_locals)]
#![feature(untagged_unions)]
#![feature(unwind_attributes)]
#![feature(variant_count)]
#![feature(doc_alias)]
#![feature(mmx_target_feature)]
#![feature(tbm_target_feature)]
Expand Down
30 changes: 30 additions & 0 deletions src/libcore/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,3 +999,33 @@ impl<T> fmt::Debug for Discriminant<T> {
pub const fn discriminant<T>(v: &T) -> Discriminant<T> {
Discriminant(intrinsics::discriminant_value(v))
}

/// Returns the number of variants in the enum type `T`.
///
/// If `T` is not an enum, calling this function will not result in undefined behavior, but the
/// return value is unspecified. Equally, if `T` is an enum with more variants than `usize::MAX`
/// the return value is unspecified. Uninhabited variants will be counted.
///
/// # Examples
///
/// ```
/// # #![feature(never_type)]
/// # #![feature(variant_count)]
///
/// use std::mem;
///
/// enum Void {}
/// enum Foo { A(&'static str), B(i32), C(i32) }
///
/// assert_eq!(mem::variant_count::<Void>(), 0);
/// assert_eq!(mem::variant_count::<Foo>(), 3);
doctorn marked this conversation as resolved.
Show resolved Hide resolved
///
/// assert_eq!(mem::variant_count::<Option<!>>(), 2);
/// assert_eq!(mem::variant_count::<Result<!, !>>(), 2);
/// ```
#[inline(always)]
#[unstable(feature = "variant_count", issue = "73662")]
#[rustc_const_unstable(feature = "variant_count", issue = "73662")]
pub const fn variant_count<T>() -> usize {
intrinsics::variant_count::<T>()
}
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
}
}
"size_of" | "pref_align_of" | "min_align_of" | "needs_drop" | "type_id"
| "type_name" => {
| "type_name" | "variant_count" => {
let value = self
.tcx
.const_eval_instance(ty::ParamEnv::reveal_all(), instance, None)
Expand Down
14 changes: 12 additions & 2 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ crate fn eval_nullary_intrinsic<'tcx>(
ConstValue::from_machine_usize(n, &tcx)
}
sym::type_id => ConstValue::from_u64(tcx.type_id_hash(tp_ty)),
sym::variant_count => {
if let ty::Adt(ref adt, _) = tp_ty.kind {
ConstValue::from_machine_usize(adt.variants.len() as u64, &tcx)
} else {
ConstValue::from_machine_usize(0u64, &tcx)
}
}
other => bug!("`{}` is not a zero arg intrinsic", other),
})
}
Expand Down Expand Up @@ -109,10 +116,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| sym::needs_drop
| sym::size_of
| sym::type_id
| sym::type_name => {
| sym::type_name
| sym::variant_count => {
let gid = GlobalId { instance, promoted: None };
let ty = match intrinsic_name {
sym::min_align_of | sym::pref_align_of | sym::size_of => self.tcx.types.usize,
sym::min_align_of | sym::pref_align_of | sym::size_of | sym::variant_count => {
self.tcx.types.usize
}
sym::needs_drop => self.tcx.types.bool,
sym::type_id => self.tcx.types.u64,
sym::type_name => self.tcx.mk_static_str(),
Expand Down
1 change: 1 addition & 0 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ symbols! {
v1,
val,
var,
variant_count,
vec,
Vec,
version,
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_typeck/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn intrinsic_operation_unsafety(intrinsic: &str) -> hir::Unsafety {
| "saturating_sub" | "rotate_left" | "rotate_right" | "ctpop" | "ctlz" | "cttz"
| "bswap" | "bitreverse" | "discriminant_value" | "type_id" | "likely" | "unlikely"
| "ptr_guaranteed_eq" | "ptr_guaranteed_ne" | "minnumf32" | "minnumf64" | "maxnumf32"
| "maxnumf64" | "type_name" => hir::Unsafety::Normal,
| "maxnumf64" | "type_name" | "variant_count" => hir::Unsafety::Normal,
_ => hir::Unsafety::Unsafe,
}
}
Expand Down Expand Up @@ -137,7 +137,9 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
let unsafety = intrinsic_operation_unsafety(&name[..]);
let (n_tps, inputs, output) = match &name[..] {
"breakpoint" => (0, Vec::new(), tcx.mk_unit()),
"size_of" | "pref_align_of" | "min_align_of" => (1, Vec::new(), tcx.types.usize),
"size_of" | "pref_align_of" | "min_align_of" | "variant_count" => {
(1, Vec::new(), tcx.types.usize)
}
"size_of_val" | "min_align_of_val" => {
(1, vec![tcx.mk_imm_ptr(param(0))], tcx.types.usize)
}
Expand Down
47 changes: 47 additions & 0 deletions src/test/ui/consts/const-variant-count.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// run-pass
#![allow(dead_code)]
#![feature(variant_count)]
#![feature(never_type)]

use std::mem::variant_count;

enum Void {}

enum Foo {
A,
B,
C,
}

enum Bar {
A,
B,
C,
D(usize),
E { field_1: usize, field_2: Foo },
}

struct Baz {
a: u32,
b: *const u8,
}

const TEST_VOID: usize = variant_count::<Void>();
const TEST_FOO: usize = variant_count::<Foo>();
const TEST_BAR: usize = variant_count::<Bar>();

const NO_ICE_STRUCT: usize = variant_count::<Baz>();
const NO_ICE_BOOL: usize = variant_count::<bool>();
const NO_ICE_PRIM: usize = variant_count::<*const u8>();

fn main() {
assert_eq!(TEST_VOID, 0);
assert_eq!(TEST_FOO, 3);
assert_eq!(TEST_BAR, 5);
assert_eq!(variant_count::<Void>(), 0);
assert_eq!(variant_count::<Foo>(), 3);
assert_eq!(variant_count::<Bar>(), 5);
assert_eq!(variant_count::<Option<char>>(), 2);
assert_eq!(variant_count::<Option<!>>(), 2);
assert_eq!(variant_count::<Result<!, !>>(), 2);
}