Skip to content

[const_panic] Report const_panic diagnostics identically to compiler_error invocations #79695

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[const_panic] Report const_panic diagnostics identically to compiler_error invocations #79695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_data_structures::sync::Lock;
use rustc_errors::{pluralize, struct_span_err, DiagnosticBuilder, ErrorReported};
use rustc_macros::HashStable;
use rustc_session::CtfeBacktrace;
use rustc_span::def_id::DefId;
use rustc_span::{def_id::DefId, Symbol};
use rustc_target::abi::{Align, Size};
use std::{any::Any, backtrace::Backtrace, fmt, mem};

Expand Down Expand Up @@ -439,6 +439,8 @@ pub enum InterpError<'tcx> {
/// The program exhausted the interpreter's resources (stack/heap too big,
/// execution takes too long, ...).
ResourceExhaustion(ResourceExhaustionInfo),
/// The program terminated immediately from a panic.
Panic(Symbol),
Copy link
Member

Choose a reason for hiding this comment

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

This is a CTFE-only error, I don't think it should be added here. This is more of a case for the MachineStopType.

/// Stop execution for a machine-controlled reason. This is never raised by
/// the core engine itself.
MachineStop(Box<dyn MachineStopType>),
Expand All @@ -454,6 +456,7 @@ impl fmt::Display for InterpError<'_> {
InvalidProgram(ref msg) => write!(f, "{}", msg),
UndefinedBehavior(ref msg) => write!(f, "{}", msg),
ResourceExhaustion(ref msg) => write!(f, "{}", msg),
Panic(ref msg) => write!(f, "{}", msg),
MachineStop(ref msg) => write!(f, "{}", msg),
}
}
Expand Down
22 changes: 16 additions & 6 deletions compiler/rustc_mir/src/const_eval/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ pub enum ConstEvalErrKind {
Panic { msg: Symbol, line: u32, col: u32, file: Symbol },
}

// The errors become `MachineStop` with plain strings when being raised.
// Non-panic errors become `MachineStop` with plain strings when being raised.
// `ConstEvalErr` (in `librustc_middle/mir/interpret/error.rs`) knows to
// handle these.
impl<'tcx> Into<InterpErrorInfo<'tcx>> for ConstEvalErrKind {
fn into(self) -> InterpErrorInfo<'tcx> {
err_machine_stop!(self.to_string()).into()
match self {
ConstEvalErrKind::Panic { msg, .. } => InterpError::Panic(msg).into(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can mirror what miri does here. Instead of having an impl MachineStopType for String, we create an

enum TerminationInfo {
    Generic(String),
    Panic(String),
}

Then you can downcast to TerminationInfo below and handle it just there.

_ => err_machine_stop!(self.to_string()).into(),
}
}
}

Expand Down Expand Up @@ -150,13 +153,14 @@ impl<'tcx> ConstEvalErr<'tcx> {
};
trace!("reporting const eval failure at {:?}", self.span);

let err_msg = match &self.error {
let (err_msg, is_panic) = match &self.error {
InterpError::MachineStop(msg) => {
// A custom error (`ConstEvalErrKind` in `librustc_mir/interp/const_eval/error.rs`).
// Should be turned into a string by now.
msg.downcast_ref::<String>().expect("invalid MachineStop payload").clone()
(msg.downcast_ref::<String>().expect("invalid MachineStop payload").clone(), false)
}
err => err.to_string(),
InterpError::Panic(msg) => (msg.to_string(), true),
err => (err.to_string(), false),
};

let finish = |mut err: DiagnosticBuilder<'_>, span_msg: Option<String>| {
Expand Down Expand Up @@ -193,7 +197,13 @@ impl<'tcx> ConstEvalErr<'tcx> {
rustc_session::lint::builtin::CONST_ERR,
hir_id,
tcx.span,
|lint| finish(lint.build(message), Some(err_msg)),
|lint| {
if is_panic {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think something similar needs to be done for the Report as hard error case below. Maybe the match &self.error above could produce a message and an Option<String> for the note, and then you can set the appropriatly once?

finish(lint.build(&err_msg), None)
} else {
finish(lint.build(message), Some(err_msg))
}
},
);
ErrorHandled::Linted
} else {
Expand Down