Skip to content

Commit

Permalink
Auto merge of rust-lang#102091 - RalfJung:const_err, r=oli-obk
Browse files Browse the repository at this point in the history
make const_err a hard error

This lint has been deny-by-default with future incompat wording since [Rust 1.51](rust-lang#80394) and the stable release of this week starts showing it in cargo's future compat reports. I can't wait to finally get rid of at least some of the mess in our const-err-reporting-code. ;)

r? `@oli-obk`
Fixes rust-lang#71800
Fixes rust-lang#100114
  • Loading branch information
bors committed Oct 7, 2022
2 parents 2d3a85b + fd59d44 commit 8b0c05d
Show file tree
Hide file tree
Showing 254 changed files with 1,379 additions and 5,321 deletions.
76 changes: 7 additions & 69 deletions compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::error::Error;
use std::fmt;

use rustc_errors::Diagnostic;
use rustc_hir as hir;
use rustc_middle::mir::AssertKind;
use rustc_middle::ty::{layout::LayoutError, query::TyCtxtAt, ConstInt};
use rustc_span::{Span, Symbol};
Expand All @@ -23,11 +22,7 @@ pub enum ConstEvalErrKind {
Abort(String),
}

impl MachineStopType for ConstEvalErrKind {
fn is_hard_err(&self) -> bool {
matches!(self, Self::Panic { .. })
}
}
impl MachineStopType for ConstEvalErrKind {}

// The errors become `MachineStop` with plain strings when being raised.
// `ConstEvalErr` (in `librustc_middle/mir/interpret/error.rs`) knows to
Expand Down Expand Up @@ -87,48 +82,10 @@ impl<'tcx> ConstEvalErr<'tcx> {
ConstEvalErr { error: error.into_kind(), stacktrace, span }
}

pub fn struct_error(
&self,
tcx: TyCtxtAt<'tcx>,
message: &str,
decorate: impl FnOnce(&mut Diagnostic),
) -> ErrorHandled {
self.struct_generic(tcx, message, decorate, None)
}

pub fn report_as_error(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled {
self.struct_error(tcx, message, |_| {})
}

pub fn report_as_lint(
&self,
tcx: TyCtxtAt<'tcx>,
message: &str,
lint_root: hir::HirId,
span: Option<Span>,
) -> ErrorHandled {
self.struct_generic(
tcx,
message,
|lint: &mut Diagnostic| {
// Apply the span.
if let Some(span) = span {
let primary_spans = lint.span.primary_spans().to_vec();
// point at the actual error as the primary span
lint.replace_span_with(span);
// point to the `const` statement as a secondary span
// they don't have any label
for sp in primary_spans {
if sp != span {
lint.span_label(sp, "");
}
}
}
},
Some(lint_root),
)
}

/// Create a diagnostic for this const eval error.
///
/// Sets the message passed in via `message` and adds span labels with detailed error
Expand All @@ -137,13 +94,12 @@ impl<'tcx> ConstEvalErr<'tcx> {
///
/// If `lint_root.is_some()` report it as a lint, else report it as a hard error.
/// (Except that for some errors, we ignore all that -- see `must_error` below.)
#[instrument(skip(self, tcx, decorate, lint_root), level = "debug")]
fn struct_generic(
#[instrument(skip(self, tcx, decorate), level = "debug")]
pub fn struct_error(
&self,
tcx: TyCtxtAt<'tcx>,
message: &str,
decorate: impl FnOnce(&mut Diagnostic),
lint_root: Option<hir::HirId>,
) -> ErrorHandled {
let finish = |err: &mut Diagnostic, span_msg: Option<String>| {
trace!("reporting const eval failure at {:?}", self.span);
Expand Down Expand Up @@ -224,27 +180,9 @@ impl<'tcx> ConstEvalErr<'tcx> {

let err_msg = self.error.to_string();

// Regular case - emit a lint.
if let Some(lint_root) = lint_root {
// Report as lint.
let hir_id =
self.stacktrace.iter().rev().find_map(|frame| frame.lint_root).unwrap_or(lint_root);
tcx.struct_span_lint_hir(
rustc_session::lint::builtin::CONST_ERR,
hir_id,
tcx.span,
message,
|lint| {
finish(lint, Some(err_msg));
lint
},
);
ErrorHandled::Linted
} else {
// Report as hard error.
let mut err = struct_error(tcx, message);
finish(&mut err, Some(err_msg));
ErrorHandled::Reported(err.emit())
}
// Report as hard error.
let mut err = struct_error(tcx, message);
finish(&mut err, Some(err_msg));
ErrorHandled::Reported(err.emit())
}
}
50 changes: 14 additions & 36 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,45 +317,23 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body)) {
Err(error) => {
let err = ConstEvalErr::new(&ecx, error, None);
// Some CTFE errors raise just a lint, not a hard error; see
// <https://github.com/rust-lang/rust/issues/71800>.
let is_hard_err = if let Some(def) = def.as_local() {
// (Associated) consts only emit a lint, since they might be unused.
!matches!(tcx.def_kind(def.did.to_def_id()), DefKind::Const | DefKind::AssocConst)
// check if the inner InterpError is hard
|| err.error.is_hard_err()
let msg = if is_static {
Cow::from("could not evaluate static initializer")
} else {
// use of broken constant from other crate: always an error
true
};

if is_hard_err {
let msg = if is_static {
Cow::from("could not evaluate static initializer")
// If the current item has generics, we'd like to enrich the message with the
// instance and its substs: to show the actual compile-time values, in addition to
// the expression, leading to the const eval error.
let instance = &key.value.instance;
if !instance.substs.is_empty() {
let instance = with_no_trimmed_paths!(instance.to_string());
let msg = format!("evaluation of `{}` failed", instance);
Cow::from(msg)
} else {
// If the current item has generics, we'd like to enrich the message with the
// instance and its substs: to show the actual compile-time values, in addition to
// the expression, leading to the const eval error.
let instance = &key.value.instance;
if !instance.substs.is_empty() {
let instance = with_no_trimmed_paths!(instance.to_string());
let msg = format!("evaluation of `{}` failed", instance);
Cow::from(msg)
} else {
Cow::from("evaluation of constant value failed")
}
};
Cow::from("evaluation of constant value failed")
}
};

Err(err.report_as_error(ecx.tcx.at(err.span), &msg))
} else {
let hir_id = tcx.hir().local_def_id_to_hir_id(def.as_local().unwrap().did);
Err(err.report_as_lint(
tcx.at(tcx.def_span(def.did)),
"any use of this value will cause an error",
hir_id,
Some(err.span),
))
}
Err(err.report_as_error(ecx.tcx.at(err.span), &msg))
}
Ok(mplace) => {
// Since evaluation had no errors, validate the resulting constant.
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ impl<'tcx> fmt::Display for FrameInfo<'tcx> {
{
write!(f, "inside closure")?;
} else {
// Note: this triggers a `good_path_bug` state, which means that if we ever get here
// we must emit a diagnostic. We should never display a `FrameInfo` unless we
// actually want to emit a warning or error to the user.
write!(f, "inside `{}`", self.instance)?;
}
if !self.span.is_dummy() {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,6 @@ pub enum InternKind {
///
/// This *cannot raise an interpreter error*. Doing so is left to validation, which
/// tracks where in the value we are and thus can show much better error messages.
/// Any errors here would anyway be turned into `const_err` lints, whereas validation failures
/// are hard errors.
#[instrument(level = "debug", skip(ecx))]
pub fn intern_const_alloc_recursive<
'mir,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,11 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
"now allowed, see issue #59159 \
<https://github.com/rust-lang/rust/issues/59159> for more information",
);
store.register_removed(
"const_err",
"converted into hard error, see issue #71800 \
<https://github.com/rust-lang/rust/issues/71800> for more information",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
32 changes: 0 additions & 32 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,37 +263,6 @@ declare_lint! {
"operation will cause a panic at runtime"
}

declare_lint! {
/// The `const_err` lint detects an erroneous expression while doing
/// constant evaluation.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![allow(unconditional_panic)]
/// const C: i32 = 1/0;
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This lint detects constants that fail to evaluate. Allowing the lint will accept the
/// constant declaration, but any use of this constant will still lead to a hard error. This is
/// a future incompatibility lint; the plan is to eventually entirely forbid even declaring
/// constants that cannot be evaluated. See [issue #71800] for more details.
///
/// [issue #71800]: https://github.com/rust-lang/rust/issues/71800
pub CONST_ERR,
Deny,
"constant evaluation encountered erroneous expression",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #71800 <https://github.com/rust-lang/rust/issues/71800>",
reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow,
};
report_in_external_macro
}

declare_lint! {
/// The `unused_imports` lint detects imports that are never used.
///
Expand Down Expand Up @@ -3295,7 +3264,6 @@ declare_lint_pass! {
EXPORTED_PRIVATE_DEPENDENCIES,
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
INVALID_TYPE_PARAM_DEFAULT,
CONST_ERR,
RENAMED_AND_REMOVED_LINTS,
UNALIGNED_REFERENCES,
CONST_ITEM_MUTATION,
Expand Down
19 changes: 1 addition & 18 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,7 @@ impl<T: Any> AsAny for T {
}

/// A trait for machine-specific errors (or other "machine stop" conditions).
pub trait MachineStopType: AsAny + fmt::Display + Send {
/// If `true`, emit a hard error instead of going through the `CONST_ERR` lint
fn is_hard_err(&self) -> bool {
false
}
}
pub trait MachineStopType: AsAny + fmt::Display + Send {}

impl dyn MachineStopType {
#[inline(always)]
Expand Down Expand Up @@ -543,16 +538,4 @@ impl InterpError<'_> {
| InterpError::UndefinedBehavior(UndefinedBehaviorInfo::Ub(_))
)
}

/// Should this error be reported as a hard error, preventing compilation, or a soft error,
/// causing a deny-by-default lint?
pub fn is_hard_err(&self) -> bool {
use InterpError::*;
match *self {
MachineStop(ref err) => err.is_hard_err(),
UndefinedBehavior(_) => true,
ResourceExhaustion(ResourceExhaustionInfo::MemoryExhausted) => true,
_ => false,
}
}
}
35 changes: 9 additions & 26 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{
self, AssertKind, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, Location, Operand,
Place, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator,
TerminatorKind, UnOp, RETURN_PLACE,
AssertKind, BinOp, Body, Constant, Local, LocalDecl, Location, Operand, Place, Rvalue,
SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind,
UnOp, RETURN_PLACE,
};
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
use rustc_middle::ty::InternalSubsts;
Expand Down Expand Up @@ -286,7 +286,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

/// Returns the value, if any, of evaluating `c`.
fn eval_constant(&mut self, c: &Constant<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
fn eval_constant(
&mut self,
c: &Constant<'tcx>,
_source_info: SourceInfo,
) -> Option<OpTy<'tcx>> {
// FIXME we need to revisit this for #67176
if c.needs_subst() {
return None;
Expand All @@ -297,28 +301,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
Err(error) => {
let tcx = self.ecx.tcx.at(c.span);
let err = ConstEvalErr::new(&self.ecx, error, Some(c.span));
if let Some(lint_root) = self.lint_root(source_info) {
let lint_only = match c.literal {
ConstantKind::Ty(ct) => ct.needs_subst(),
ConstantKind::Unevaluated(
mir::UnevaluatedConst { def: _, substs: _, promoted: Some(_) },
_,
) => {
// Promoteds must lint and not error as the user didn't ask for them
true
}
ConstantKind::Unevaluated(..) | ConstantKind::Val(..) => c.needs_subst(),
};
if lint_only {
// Out of backwards compatibility we cannot report hard errors in unused
// generic functions using associated constants of the generic parameters.
err.report_as_lint(tcx, "erroneous constant used", lint_root, Some(c.span));
} else {
err.report_as_error(tcx, "erroneous constant used");
}
} else {
err.report_as_error(tcx, "erroneous constant used");
}
err.report_as_error(tcx, "erroneous constant used");
None
}
}
Expand Down
2 changes: 0 additions & 2 deletions library/core/tests/num/wrapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ wrapping_test!(test_wrapping_u64, u64, u64::MIN, u64::MAX);
wrapping_test!(test_wrapping_u128, u128, u128::MIN, u128::MAX);
wrapping_test!(test_wrapping_usize, usize, usize::MIN, usize::MAX);

// Don't warn about overflowing ops on 32-bit platforms
#[cfg_attr(target_pointer_width = "32", allow(const_err))]
#[test]
fn wrapping_int_api() {
assert_eq!(i8::MAX.wrapping_add(1), i8::MIN);
Expand Down
1 change: 0 additions & 1 deletion library/core/tests/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,6 @@ fn test_windows_zip() {
}

#[test]
#[allow(const_err)]
fn test_iter_ref_consistency() {
use std::fmt::Debug;

Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/remove-never-const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
// compile-flags: --emit mir,link

#![feature(never_type)]
#![warn(const_err)]

struct PrintName<T>(T);

Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/array-slice-vec/array_const_index-0.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const A: &'static [i32] = &[];
const B: i32 = (&A)[1];
//~^ index out of bounds: the length is 0 but the index is 1
//~| ERROR any use of this value will cause an error
//~| WARN this was previously accepted by the compiler but is being phased out
//~| ERROR evaluation of constant value failed

fn main() {
let _ = B;
Expand Down
Loading

0 comments on commit 8b0c05d

Please sign in to comment.