Skip to content

Commit e2be5f5

Browse files
committed
Auto merge of #74595 - lcnr:ConstEvaluatable-fut-compat, r=oli-obk
make `ConstEvaluatable` more strict relevant zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/.60ConstEvaluatable.60.20generic.20functions/near/204125452 Let's see how much this impacts. Depending on how this goes this should probably be a future compat warning. Short explanation: we currently forbid anonymous constants which depend on generic types, e.g. `[0; std::mem::size_of::<T>]` currently errors. We previously checked this by evaluating the constant and returned an error if that failed. This however allows things like ```rust const fn foo<T>() -> usize { if std::mem::size_of::<*mut T>() < 8 { // size of *mut T does not depend on T std::mem::size_of::<T>() } else { 8 } } fn test<T>() { let _ = [0; foo::<T>()]; } ``` which is a backwards compatibility hazard. This also has worrying interactions with mir optimizations (#74491 (comment)) and intrinsics (#74538). r? `@oli-obk` `@eddyb`
2 parents d92155b + 74e0719 commit e2be5f5

File tree

14 files changed

+184
-18
lines changed

14 files changed

+184
-18
lines changed

compiler/rustc_middle/src/mir/mod.rs

+27-4
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,23 @@ pub struct Body<'tcx> {
186186
/// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components.
187187
pub ignore_interior_mut_in_const_validation: bool,
188188

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

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

211-
Body {
228+
let mut body = Body {
212229
phase: MirPhase::Build,
213230
basic_blocks,
214231
source_scopes,
@@ -224,8 +241,11 @@ impl<'tcx> Body<'tcx> {
224241
span,
225242
required_consts: Vec::new(),
226243
ignore_interior_mut_in_const_validation: false,
244+
is_polymorphic: false,
227245
predecessor_cache: PredecessorCache::new(),
228-
}
246+
};
247+
body.is_polymorphic = body.has_param_types_or_consts();
248+
body
229249
}
230250

231251
/// Returns a partially initialized MIR body containing only a list of basic blocks.
@@ -234,7 +254,7 @@ impl<'tcx> Body<'tcx> {
234254
/// is only useful for testing but cannot be `#[cfg(test)]` because it is used in a different
235255
/// crate.
236256
pub fn new_cfg_only(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) -> Self {
237-
Body {
257+
let mut body = Body {
238258
phase: MirPhase::Build,
239259
basic_blocks,
240260
source_scopes: IndexVec::new(),
@@ -250,8 +270,11 @@ impl<'tcx> Body<'tcx> {
250270
generator_kind: None,
251271
var_debug_info: Vec::new(),
252272
ignore_interior_mut_in_const_validation: false,
273+
is_polymorphic: false,
253274
predecessor_cache: PredecessorCache::new(),
254-
}
275+
};
276+
body.is_polymorphic = body.has_param_types_or_consts();
277+
body
255278
}
256279

257280
#[inline]

compiler/rustc_session/src/lint/builtin.rs

+11
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,16 @@ declare_lint! {
539539
};
540540
}
541541

542+
declare_lint! {
543+
pub CONST_EVALUATABLE_UNCHECKED,
544+
Warn,
545+
"detects a generic constant is used in a type without a emitting a warning",
546+
@future_incompatible = FutureIncompatibleInfo {
547+
reference: "issue #76200 <https://github.com/rust-lang/rust/issues/76200>",
548+
edition: None,
549+
};
550+
}
551+
542552
declare_lint_pass! {
543553
/// Does nothing as a lint pass, but registers some `Lint`s
544554
/// that are used by other parts of the compiler.
@@ -612,6 +622,7 @@ declare_lint_pass! {
612622
UNSAFE_OP_IN_UNSAFE_FN,
613623
INCOMPLETE_INCLUDE,
614624
CENUM_IMPL_DROP_CAST,
625+
CONST_EVALUATABLE_UNCHECKED,
615626
]
616627
}
617628

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
use rustc_hir::def::DefKind;
2+
use rustc_infer::infer::InferCtxt;
3+
use rustc_middle::mir::interpret::ErrorHandled;
4+
use rustc_middle::ty::subst::SubstsRef;
5+
use rustc_middle::ty::{self, TypeFoldable};
6+
use rustc_session::lint;
7+
use rustc_span::def_id::DefId;
8+
use rustc_span::Span;
9+
10+
pub fn is_const_evaluatable<'cx, 'tcx>(
11+
infcx: &InferCtxt<'cx, 'tcx>,
12+
def: ty::WithOptConstParam<DefId>,
13+
substs: SubstsRef<'tcx>,
14+
param_env: ty::ParamEnv<'tcx>,
15+
span: Span,
16+
) -> Result<(), ErrorHandled> {
17+
let future_compat_lint = || {
18+
if let Some(local_def_id) = def.did.as_local() {
19+
infcx.tcx.struct_span_lint_hir(
20+
lint::builtin::CONST_EVALUATABLE_UNCHECKED,
21+
infcx.tcx.hir().local_def_id_to_hir_id(local_def_id),
22+
span,
23+
|err| {
24+
err.build("cannot use constants which depend on generic parameters in types")
25+
.emit();
26+
},
27+
);
28+
}
29+
};
30+
31+
// FIXME: We should only try to evaluate a given constant here if it is fully concrete
32+
// as we don't want to allow things like `[u8; std::mem::size_of::<*mut T>()]`.
33+
//
34+
// We previously did not check this, so we only emit a future compat warning if
35+
// const evaluation succeeds and the given constant is still polymorphic for now
36+
// and hopefully soon change this to an error.
37+
//
38+
// See #74595 for more details about this.
39+
let concrete = infcx.const_eval_resolve(param_env, def, substs, None, Some(span));
40+
41+
let def_kind = infcx.tcx.def_kind(def.did);
42+
match def_kind {
43+
DefKind::AnonConst => {
44+
let mir_body = if let Some(def) = def.as_const_arg() {
45+
infcx.tcx.optimized_mir_of_const_arg(def)
46+
} else {
47+
infcx.tcx.optimized_mir(def.did)
48+
};
49+
if mir_body.is_polymorphic && concrete.is_ok() {
50+
future_compat_lint();
51+
}
52+
}
53+
_ => {
54+
if substs.has_param_types_or_consts() && concrete.is_ok() {
55+
future_compat_lint();
56+
}
57+
}
58+
}
59+
60+
concrete.map(drop)
61+
}

compiler/rustc_trait_selection/src/traits/fulfill.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_middle::ty::ToPredicate;
1010
use rustc_middle::ty::{self, Binder, Const, Ty, TypeFoldable};
1111
use std::marker::PhantomData;
1212

13+
use super::const_evaluatable;
1314
use super::project;
1415
use super::select::SelectionContext;
1516
use super::wf;
@@ -458,15 +459,15 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
458459
}
459460

460461
ty::PredicateAtom::ConstEvaluatable(def_id, substs) => {
461-
match self.selcx.infcx().const_eval_resolve(
462-
obligation.param_env,
462+
match const_evaluatable::is_const_evaluatable(
463+
self.selcx.infcx(),
463464
def_id,
464465
substs,
465-
None,
466-
Some(obligation.cause.span),
466+
obligation.param_env,
467+
obligation.cause.span,
467468
) {
468-
Ok(_) => ProcessResult::Changed(vec![]),
469-
Err(err) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(err))),
469+
Ok(()) => ProcessResult::Changed(vec![]),
470+
Err(e) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(e))),
470471
}
471472
}
472473

compiler/rustc_trait_selection/src/traits/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub mod auto_trait;
77
mod chalk_fulfill;
88
pub mod codegen;
99
mod coherence;
10+
mod const_evaluatable;
1011
mod engine;
1112
pub mod error_reporting;
1213
mod fulfill;

compiler/rustc_trait_selection/src/traits/select/mod.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use self::EvaluationResult::*;
66
use self::SelectionCandidate::*;
77

88
use super::coherence::{self, Conflict};
9+
use super::const_evaluatable;
910
use super::project;
1011
use super::project::normalize_with_depth_to;
1112
use super::util;
@@ -542,14 +543,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
542543
}
543544

544545
ty::PredicateAtom::ConstEvaluatable(def_id, substs) => {
545-
match self.tcx().const_eval_resolve(
546-
obligation.param_env,
546+
match const_evaluatable::is_const_evaluatable(
547+
self.infcx,
547548
def_id,
548549
substs,
549-
None,
550-
None,
550+
obligation.param_env,
551+
obligation.cause.span,
551552
) {
552-
Ok(_) => Ok(EvaluatedToOk),
553+
Ok(()) => Ok(EvaluatedToOk),
553554
Err(ErrorHandled::TooGeneric) => Ok(EvaluatedToAmbig),
554555
Err(_) => Ok(EvaluatedToErr),
555556
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// check-pass
2+
struct Foo<T>(T);
3+
impl<T> Foo<T> {
4+
const VALUE: usize = std::mem::size_of::<T>();
5+
}
6+
7+
fn test<T>() {
8+
let _ = [0; Foo::<u8>::VALUE];
9+
}
10+
11+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// check-pass
2+
3+
const fn foo<T>() -> usize {
4+
// We might instead branch on `std::mem::size_of::<*mut T>() < 8` here,
5+
// which would cause this function to fail on 32 bit systems.
6+
if false {
7+
std::mem::size_of::<T>()
8+
} else {
9+
8
10+
}
11+
}
12+
13+
fn test<T>() {
14+
let _ = [0; foo::<T>()];
15+
//~^ WARN cannot use constants which depend on generic parameters in types
16+
//~| WARN this was previously accepted by the compiler but is being phased out
17+
}
18+
19+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
warning: cannot use constants which depend on generic parameters in types
2+
--> $DIR/function-call.rs:14:17
3+
|
4+
LL | let _ = [0; foo::<T>()];
5+
| ^^^^^^^^^^
6+
|
7+
= note: `#[warn(const_evaluatable_unchecked)]` on by default
8+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
9+
= note: for more information, see issue #76200 <https://github.com/rust-lang/rust/issues/76200>
10+
11+
warning: 1 warning emitted
12+

src/test/ui/enum-discriminant/issue-70453-polymorphic-ctfe.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// run-pass
2-
32
#![feature(arbitrary_enum_discriminant, core_intrinsics)]
43

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

1415
fn main() {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
warning: cannot use constants which depend on generic parameters in types
2+
--> $DIR/issue-70453-polymorphic-ctfe.rs:10:15
3+
|
4+
LL | Some(T) = core::mem::size_of::<*mut T>(),
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(const_evaluatable_unchecked)]` on by default
8+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
9+
= note: for more information, see issue #76200 <https://github.com/rust-lang/rust/issues/76200>
10+
11+
warning: 1 warning emitted
12+

src/test/ui/impl-trait/issue-56445.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55

66
use std::marker::PhantomData;
77

8-
pub struct S<'a>
9-
{
8+
pub struct S<'a> {
109
pub m1: PhantomData<&'a u8>,
1110
pub m2: [u8; S::size()],
1211
}

src/test/ui/lazy_normalization_consts/issue-73980.rs

+2
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,7 @@ impl<T: ?Sized> L<T> {
1010
}
1111

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

1416
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
warning: cannot use constants which depend on generic parameters in types
2+
--> $DIR/issue-73980.rs:12:9
3+
|
4+
LL | impl<T> X<T, [u8; L::<T>::S]> {}
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(const_evaluatable_unchecked)]` on by default
8+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
9+
= note: for more information, see issue #76200 <https://github.com/rust-lang/rust/issues/76200>
10+
11+
warning: 1 warning emitted
12+

0 commit comments

Comments
 (0)