Skip to content

make ConstEvaluatable more strict #74595

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

Merged
merged 7 commits into from
Sep 9, 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
31 changes: 27 additions & 4 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,23 @@ pub struct Body<'tcx> {
/// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components.
pub ignore_interior_mut_in_const_validation: bool,

/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
///
/// Note that this does not actually mean that this body is not computable right now.
/// The repeat count in the following example is polymorphic, but can still be evaluated
/// without knowing anything about the type parameter `T`.
///
/// ```rust
/// fn test<T>() {
/// let _ = [0; std::mem::size_of::<*mut T>()];
/// }
/// ```
///
/// **WARNING**: Do not change this flags after the MIR was originally created, even if an optimization
/// removed the last mention of all generic params. We do not want to rely on optimizations and
/// potentially allow things like `[u8; std::mem::size_of::<T>() * 0]` due to this.
pub is_polymorphic: bool,

predecessor_cache: PredecessorCache,
}

Expand All @@ -208,7 +225,7 @@ impl<'tcx> Body<'tcx> {
local_decls.len()
);

Body {
let mut body = Body {
phase: MirPhase::Build,
basic_blocks,
source_scopes,
Expand All @@ -224,8 +241,11 @@ impl<'tcx> Body<'tcx> {
span,
required_consts: Vec::new(),
ignore_interior_mut_in_const_validation: false,
is_polymorphic: false,
predecessor_cache: PredecessorCache::new(),
}
};
body.is_polymorphic = body.has_param_types_or_consts();
body
}

/// Returns a partially initialized MIR body containing only a list of basic blocks.
Expand All @@ -234,7 +254,7 @@ impl<'tcx> Body<'tcx> {
/// is only useful for testing but cannot be `#[cfg(test)]` because it is used in a different
/// crate.
pub fn new_cfg_only(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) -> Self {
Body {
let mut body = Body {
phase: MirPhase::Build,
basic_blocks,
source_scopes: IndexVec::new(),
Expand All @@ -250,8 +270,11 @@ impl<'tcx> Body<'tcx> {
generator_kind: None,
var_debug_info: Vec::new(),
ignore_interior_mut_in_const_validation: false,
is_polymorphic: false,
predecessor_cache: PredecessorCache::new(),
}
};
body.is_polymorphic = body.has_param_types_or_consts();
body
}

#[inline]
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_session/src/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,16 @@ declare_lint! {
};
}

declare_lint! {
pub CONST_EVALUATABLE_UNCHECKED,
Warn,
"detects a generic constant is used in a type without a emitting a warning",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #76200 <https://github.com./rust-lang/rust/issues/76200>",
edition: None,
};
}

declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
/// that are used by other parts of the compiler.
Expand Down Expand Up @@ -612,6 +622,7 @@ declare_lint_pass! {
UNSAFE_OP_IN_UNSAFE_FN,
INCOMPLETE_INCLUDE,
CENUM_IMPL_DROP_CAST,
CONST_EVALUATABLE_UNCHECKED,
]
}

Expand Down
61 changes: 61 additions & 0 deletions compiler/rustc_trait_selection/src/traits/const_evaluatable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use rustc_hir::def::DefKind;
use rustc_infer::infer::InferCtxt;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, TypeFoldable};
use rustc_session::lint;
use rustc_span::def_id::DefId;
use rustc_span::Span;

pub fn is_const_evaluatable<'cx, 'tcx>(
infcx: &InferCtxt<'cx, 'tcx>,
def: ty::WithOptConstParam<DefId>,
substs: SubstsRef<'tcx>,
param_env: ty::ParamEnv<'tcx>,
span: Span,
) -> Result<(), ErrorHandled> {
let future_compat_lint = || {
if let Some(local_def_id) = def.did.as_local() {
infcx.tcx.struct_span_lint_hir(
lint::builtin::CONST_EVALUATABLE_UNCHECKED,
infcx.tcx.hir().local_def_id_to_hir_id(local_def_id),
span,
|err| {
err.build("cannot use constants which depend on generic parameters in types")
.emit();
},
);
}
};

// FIXME: We should only try to evaluate a given constant here if it is fully concrete
// as we don't want to allow things like `[u8; std::mem::size_of::<*mut T>()]`.
//
// We previously did not check this, so we only emit a future compat warning if
// const evaluation succeeds and the given constant is still polymorphic for now
// and hopefully soon change this to an error.
//
// See #74595 for more details about this.
let concrete = infcx.const_eval_resolve(param_env, def, substs, None, Some(span));

let def_kind = infcx.tcx.def_kind(def.did);
match def_kind {
DefKind::AnonConst => {
let mir_body = if let Some(def) = def.as_const_arg() {
infcx.tcx.optimized_mir_of_const_arg(def)
} else {
infcx.tcx.optimized_mir(def.did)
};
if mir_body.is_polymorphic && concrete.is_ok() {
future_compat_lint();
}
}
_ => {
if substs.has_param_types_or_consts() && concrete.is_ok() {
future_compat_lint();
}
}
}

concrete.map(drop)
}
13 changes: 7 additions & 6 deletions compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_middle::ty::ToPredicate;
use rustc_middle::ty::{self, Binder, Const, Ty, TypeFoldable};
use std::marker::PhantomData;

use super::const_evaluatable;
use super::project;
use super::select::SelectionContext;
use super::wf;
Expand Down Expand Up @@ -458,15 +459,15 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
}

ty::PredicateAtom::ConstEvaluatable(def_id, substs) => {
match self.selcx.infcx().const_eval_resolve(
obligation.param_env,
match const_evaluatable::is_const_evaluatable(
self.selcx.infcx(),
def_id,
substs,
None,
Some(obligation.cause.span),
obligation.param_env,
obligation.cause.span,
) {
Ok(_) => ProcessResult::Changed(vec![]),
Err(err) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(err))),
Ok(()) => ProcessResult::Changed(vec![]),
Err(e) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(e))),
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod auto_trait;
mod chalk_fulfill;
pub mod codegen;
mod coherence;
mod const_evaluatable;
mod engine;
pub mod error_reporting;
mod fulfill;
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use self::EvaluationResult::*;
use self::SelectionCandidate::*;

use super::coherence::{self, Conflict};
use super::const_evaluatable;
use super::project;
use super::project::normalize_with_depth_to;
use super::util;
Expand Down Expand Up @@ -542,14 +543,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

ty::PredicateAtom::ConstEvaluatable(def_id, substs) => {
match self.tcx().const_eval_resolve(
obligation.param_env,
match const_evaluatable::is_const_evaluatable(
self.infcx,
def_id,
substs,
None,
None,
obligation.param_env,
obligation.cause.span,
) {
Ok(_) => Ok(EvaluatedToOk),
Ok(()) => Ok(EvaluatedToOk),
Err(ErrorHandled::TooGeneric) => Ok(EvaluatedToAmbig),
Err(_) => Ok(EvaluatedToErr),
}
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/const_evaluatable/associated-const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// check-pass
struct Foo<T>(T);
impl<T> Foo<T> {
const VALUE: usize = std::mem::size_of::<T>();
}

fn test<T>() {
let _ = [0; Foo::<u8>::VALUE];
}

fn main() {}
19 changes: 19 additions & 0 deletions src/test/ui/const_evaluatable/function-call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// check-pass

const fn foo<T>() -> usize {
// We might instead branch on `std::mem::size_of::<*mut T>() < 8` here,
// which would cause this function to fail on 32 bit systems.
if false {
std::mem::size_of::<T>()
} else {
8
}
}

fn test<T>() {
let _ = [0; foo::<T>()];
//~^ WARN cannot use constants which depend on generic parameters in types
//~| WARN this was previously accepted by the compiler but is being phased out
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/const_evaluatable/function-call.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
warning: cannot use constants which depend on generic parameters in types
--> $DIR/function-call.rs:14:17
|
LL | let _ = [0; foo::<T>()];
| ^^^^^^^^^^
|
= note: `#[warn(const_evaluatable_unchecked)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #76200 <https://github.com./rust-lang/rust/issues/76200>

warning: 1 warning emitted

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// run-pass

#![feature(arbitrary_enum_discriminant, core_intrinsics)]

extern crate core;
Expand All @@ -9,6 +8,8 @@ use core::intrinsics::discriminant_value;
enum MyWeirdOption<T> {
None = 0,
Some(T) = core::mem::size_of::<*mut T>(),
//~^ WARN cannot use constants which depend on generic parameters in types
//~| WARN this was previously accepted by the compiler but is being phased out
}

fn main() {
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/enum-discriminant/issue-70453-polymorphic-ctfe.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
warning: cannot use constants which depend on generic parameters in types
--> $DIR/issue-70453-polymorphic-ctfe.rs:10:15
|
LL | Some(T) = core::mem::size_of::<*mut T>(),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(const_evaluatable_unchecked)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #76200 <https://github.com./rust-lang/rust/issues/76200>

warning: 1 warning emitted

3 changes: 1 addition & 2 deletions src/test/ui/impl-trait/issue-56445.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@

use std::marker::PhantomData;

pub struct S<'a>
{
pub struct S<'a> {
pub m1: PhantomData<&'a u8>,
pub m2: [u8; S::size()],
}
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/lazy_normalization_consts/issue-73980.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ impl<T: ?Sized> L<T> {
}

impl<T> X<T, [u8; L::<T>::S]> {}
//~^ WARN cannot use constants which depend on generic parameters
//~| WARN this was previously accepted by the compiler but is being phased out

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/lazy_normalization_consts/issue-73980.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
warning: cannot use constants which depend on generic parameters in types
--> $DIR/issue-73980.rs:12:9
|
LL | impl<T> X<T, [u8; L::<T>::S]> {}
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(const_evaluatable_unchecked)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #76200 <https://github.com./rust-lang/rust/issues/76200>

warning: 1 warning emitted