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

Organize intrinsics promotion checks #66275

Merged
merged 8 commits into from
Dec 4, 2019
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
1 change: 1 addition & 0 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,7 @@ extern "rust-intrinsic" {
/// }
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_transmute")]
pub fn transmute<T, U>(e: T) -> U;
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

/// Returns `true` if the actual type given as `T` requires drop
Expand Down
115 changes: 112 additions & 3 deletions src/librustc/ty/constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use crate::ty::query::Providers;
use crate::hir::def_id::DefId;
use crate::hir;
use crate::ty::TyCtxt;
use syntax_pos::symbol::Symbol;
use syntax_pos::symbol::{sym, Symbol};
use rustc_target::spec::abi::Abi;
use crate::hir::map::blocks::FnLikeNode;
use syntax::attr;

Expand Down Expand Up @@ -35,12 +36,51 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// Returns `true` if the `def_id` refers to an intrisic which we've whitelisted
/// for being called from stable `const fn`s (`min_const_fn`).
///
/// Adding more intrinsics requires sign-off from @rust-lang/lang.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
///
/// This list differs from the list in `is_const_intrinsic` in the sense that any item on this
/// list must be on the `is_const_intrinsic` list, too, because if an intrinsic is callable from
/// stable, it must be callable at all.
fn is_intrinsic_min_const_fn(self, def_id: DefId) -> bool {
match self.item_name(def_id) {
| sym::size_of
| sym::min_align_of
| sym::needs_drop
// Arithmetic:
| sym::add_with_overflow // ~> .overflowing_add
| sym::sub_with_overflow // ~> .overflowing_sub
| sym::mul_with_overflow // ~> .overflowing_mul
| sym::wrapping_add // ~> .wrapping_add
| sym::wrapping_sub // ~> .wrapping_sub
| sym::wrapping_mul // ~> .wrapping_mul
| sym::saturating_add // ~> .saturating_add
| sym::saturating_sub // ~> .saturating_sub
| sym::unchecked_shl // ~> .wrapping_shl
| sym::unchecked_shr // ~> .wrapping_shr
| sym::rotate_left // ~> .rotate_left
| sym::rotate_right // ~> .rotate_right
| sym::ctpop // ~> .count_ones
| sym::ctlz // ~> .leading_zeros
| sym::cttz // ~> .trailing_zeros
| sym::bswap // ~> .swap_bytes
| sym::bitreverse // ~> .reverse_bits
=> true,
_ => false,
}
}

/// Returns `true` if this function must conform to `min_const_fn`
pub fn is_min_const_fn(self, def_id: DefId) -> bool {
// Bail out if the signature doesn't contain `const`
if !self.is_const_fn_raw(def_id) {
return false;
}
if let Abi::RustIntrinsic = self.fn_sig(def_id).abi() {
return self.is_intrinsic_min_const_fn(def_id);
}

if self.features().staged_api {
// in order for a libstd function to be considered min_const_fn
Expand All @@ -63,13 +103,82 @@ impl<'tcx> TyCtxt<'tcx> {


pub fn provide(providers: &mut Providers<'_>) {
/// only checks whether the function has a `const` modifier
/// Const evaluability whitelist is here to check evaluability at the
/// top level beforehand.
fn is_const_intrinsic(tcx: TyCtxt<'_>, def_id: DefId) -> Option<bool> {
match tcx.fn_sig(def_id).abi() {
Abi::RustIntrinsic |
Abi::PlatformIntrinsic => {
// FIXME: deduplicate these two lists as much as possible
match tcx.item_name(def_id) {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we de-dup this list with the min-const-fn whitelist by having this one first consult the min-const-fn list?

Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk said he'd do some clean-up in a follow-up PR. For now we just want to get our const check in order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave a FIXME tho?

// Keep this list in the same order as the match patterns in
// `librustc_mir/interpret/intrinsics.rs`

// This whitelist is a list of intrinsics that have a miri-engine implementation
// and can thus be called when enabling enough feature gates. The similar
// whitelist in `is_intrinsic_min_const_fn` (in this file), exists for allowing
// the intrinsics to be called by stable const fns.
| sym::caller_location

| sym::min_align_of
| sym::pref_align_of
| sym::needs_drop
| sym::size_of
| sym::type_id
| sym::type_name

| sym::ctpop
| sym::cttz
| sym::cttz_nonzero
| sym::ctlz
| sym::ctlz_nonzero
| sym::bswap
| sym::bitreverse

| sym::wrapping_add
| sym::wrapping_sub
| sym::wrapping_mul
| sym::add_with_overflow
| sym::sub_with_overflow
| sym::mul_with_overflow

| sym::saturating_add
| sym::saturating_sub

| sym::unchecked_shl
| sym::unchecked_shr

| sym::rotate_left
| sym::rotate_right

| sym::ptr_offset_from

| sym::transmute

| sym::simd_insert

| sym::simd_extract

=> Some(true),

_ => Some(false)
}
}
_ => None
}
}

/// Checks whether the function has a `const` modifier or, in case it is an intrinsic, whether
/// said intrinsic is on the whitelist for being const callable.
fn is_const_fn_raw(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
let hir_id = tcx.hir().as_local_hir_id(def_id)
.expect("Non-local call to local provider is_const_fn");

let node = tcx.hir().get(hir_id);
if let Some(fn_like) = FnLikeNode::from_node(node) {

if let Some(whitelisted) = is_const_intrinsic(tcx, def_id) {
whitelisted
} else if let Some(fn_like) = FnLikeNode::from_node(node) {
fn_like.constness() == hir::Constness::Const
} else if let hir::Node::Ctor(_) = node {
true
Expand Down
4 changes: 0 additions & 4 deletions src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,6 @@ declare_features! (
/// Allows using `#[doc(keyword = "...")]`.
(active, doc_keyword, "1.28.0", Some(51315), None),

/// Allows reinterpretation of the bits of a value of one type as another
/// type during const eval.
(active, const_transmute, "1.29.0", Some(53605), None),

/// Allows using `try {...}` expressions.
(active, try_blocks, "1.29.0", Some(31436), None),

Expand Down
6 changes: 6 additions & 0 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,10 +1360,16 @@ impl<'a, 'tcx> CrateMetadata {
}
}

// This replicates some of the logic of the crate-local `is_const_fn_raw` query, because we
// don't serialize constness for tuple variant and tuple struct constructors.
fn is_const_fn_raw(&self, id: DefIndex) -> bool {
let constness = match self.kind(id) {
EntryKind::Method(data) => data.decode(self).fn_data.constness,
EntryKind::Fn(data) => data.decode(self).constness,
// Some intrinsics can be const fn. While we could recompute this (at least until we
// stop having hardcoded whitelists and move to stability attributes), it seems cleaner
// to treat all const fns equally.
EntryKind::ForeignFn(data) => data.decode(self).constness,
EntryKind::Variant(..) | EntryKind::Struct(..) => hir::Constness::Const,
_ => hir::Constness::NotConst,
};
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,11 @@ impl EncodeContext<'tcx> {
hir::ForeignItemKind::Fn(_, ref names, _) => {
let data = FnData {
asyncness: hir::IsAsync::NotAsync,
constness: hir::Constness::NotConst,
constness: if self.tcx.is_const_fn_raw(def_id) {
hir::Constness::Const
} else {
hir::Constness::NotConst
},
param_names: self.encode_fn_param_names(names),
};
EntryKind::ForeignFn(self.lazy(data))
Expand Down
Loading