-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add ConstKind::Error
and convert ErrorHandled::Reported
to it.
#71049
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
Changes from all commits
2deb39d
e22d479
d7c4081
77f38dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ use crate::ty::{self, layout, Ty}; | |
|
||
use backtrace::Backtrace; | ||
use rustc_data_structures::sync::Lock; | ||
use rustc_errors::{struct_span_err, DiagnosticBuilder}; | ||
use rustc_errors::{struct_span_err, DiagnosticBuilder, ErrorReported}; | ||
use rustc_hir as hir; | ||
use rustc_hir::definitions::DefPathData; | ||
use rustc_macros::HashStable; | ||
|
@@ -19,25 +19,16 @@ use std::{any::Any, fmt, mem}; | |
|
||
#[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable, RustcEncodable, RustcDecodable)] | ||
pub enum ErrorHandled { | ||
/// Already reported a lint or an error for this evaluation. | ||
Reported, | ||
/// Already reported an error for this evaluation, and the compilation is | ||
/// *guaranteed* to fail. Warnings/lints *must not* produce `Reported`. | ||
Reported(ErrorReported), | ||
/// Already emitted a lint for this evaluation. | ||
Linted, | ||
/// Don't emit an error, the evaluation failed because the MIR was generic | ||
/// and the substs didn't fully monomorphize it. | ||
TooGeneric, | ||
} | ||
|
||
impl ErrorHandled { | ||
pub fn assert_reported(self) { | ||
match self { | ||
ErrorHandled::Reported => {} | ||
ErrorHandled::TooGeneric => bug!( | ||
"MIR interpretation failed without reporting an error \ | ||
even though it was fully monomorphized" | ||
), | ||
} | ||
} | ||
} | ||
|
||
CloneTypeFoldableImpls! { | ||
ErrorHandled, | ||
} | ||
|
@@ -84,15 +75,12 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
tcx: TyCtxtAt<'tcx>, | ||
message: &str, | ||
emit: impl FnOnce(DiagnosticBuilder<'_>), | ||
) -> Result<(), ErrorHandled> { | ||
) -> ErrorHandled { | ||
self.struct_generic(tcx, message, emit, None) | ||
} | ||
|
||
pub fn report_as_error(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled { | ||
match self.struct_error(tcx, message, |mut e| e.emit()) { | ||
Ok(_) => ErrorHandled::Reported, | ||
Err(x) => x, | ||
} | ||
self.struct_error(tcx, message, |mut e| e.emit()) | ||
} | ||
|
||
pub fn report_as_lint( | ||
|
@@ -102,7 +90,7 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
lint_root: hir::HirId, | ||
span: Option<Span>, | ||
) -> ErrorHandled { | ||
match self.struct_generic( | ||
self.struct_generic( | ||
tcx, | ||
message, | ||
|mut lint: DiagnosticBuilder<'_>| { | ||
|
@@ -122,10 +110,7 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
lint.emit(); | ||
}, | ||
Some(lint_root), | ||
) { | ||
Ok(_) => ErrorHandled::Reported, | ||
Err(err) => err, | ||
} | ||
) | ||
} | ||
|
||
/// Create a diagnostic for this const eval error. | ||
|
@@ -143,12 +128,14 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
message: &str, | ||
emit: impl FnOnce(DiagnosticBuilder<'_>), | ||
lint_root: Option<hir::HirId>, | ||
) -> Result<(), ErrorHandled> { | ||
) -> ErrorHandled { | ||
let must_error = match self.error { | ||
err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => { | ||
return Err(ErrorHandled::TooGeneric); | ||
return ErrorHandled::TooGeneric; | ||
} | ||
err_inval!(TypeckError(error_reported)) => { | ||
return ErrorHandled::Reported(error_reported); | ||
} | ||
err_inval!(TypeckError) => return Err(ErrorHandled::Reported), | ||
// We must *always* hard error on these, even if the caller wants just a lint. | ||
err_inval!(Layout(LayoutError::SizeOverflow(_))) => true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this case doesn't early return so it results in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function unfortunately became a mess over time. :/ I get the feeling we are trying to shove too many different things into one code path here. I also do not have the slightest clue what that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My thoughts exactly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, so the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that works, too, lgtm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoiding that boolean could have made the code cleaner... but maybe not, and anyway we can leave that to later. |
||
_ => false, | ||
|
@@ -183,6 +170,7 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
// caller thinks anyway. | ||
// See <https://github.com/rust-lang/rust/pull/63152>. | ||
finish(struct_error(tcx, &err_msg), None); | ||
ErrorHandled::Reported(ErrorReported) | ||
} else { | ||
// Regular case. | ||
if let Some(lint_root) = lint_root { | ||
|
@@ -200,12 +188,13 @@ impl<'tcx> ConstEvalErr<'tcx> { | |
tcx.span, | ||
|lint| finish(lint.build(message), Some(err_msg)), | ||
); | ||
ErrorHandled::Linted | ||
} else { | ||
// Report as hard error. | ||
finish(struct_error(tcx, message), Some(err_msg)); | ||
ErrorHandled::Reported(ErrorReported) | ||
} | ||
} | ||
Ok(()) | ||
} | ||
} | ||
|
||
|
@@ -246,7 +235,9 @@ fn print_backtrace(backtrace: &mut Backtrace) { | |
impl From<ErrorHandled> for InterpErrorInfo<'_> { | ||
fn from(err: ErrorHandled) -> Self { | ||
match err { | ||
ErrorHandled::Reported => err_inval!(ReferencedConstant), | ||
ErrorHandled::Reported(ErrorReported) | ErrorHandled::Linted => { | ||
err_inval!(ReferencedConstant) | ||
} | ||
ErrorHandled::TooGeneric => err_inval!(TooGeneric), | ||
} | ||
.into() | ||
|
@@ -288,7 +279,7 @@ pub enum InvalidProgramInfo<'tcx> { | |
/// which already produced an error. | ||
ReferencedConstant, | ||
/// Abort in case type errors are reached. | ||
TypeckError, | ||
TypeckError(ErrorReported), | ||
/// An error occurred during layout computation. | ||
Layout(layout::LayoutError<'tcx>), | ||
/// An invalid transmute happened. | ||
|
@@ -301,7 +292,9 @@ impl fmt::Debug for InvalidProgramInfo<'_> { | |
match self { | ||
TooGeneric => write!(f, "encountered overly generic constant"), | ||
ReferencedConstant => write!(f, "referenced constant has errors"), | ||
TypeckError => write!(f, "encountered constants with type errors, stopping evaluation"), | ||
TypeckError(ErrorReported) => { | ||
write!(f, "encountered constants with type errors, stopping evaluation") | ||
} | ||
Layout(ref err) => write!(f, "{}", err), | ||
TransmuteSizeDiff(from_ty, to_ty) => write!( | ||
f, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -510,12 +510,21 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>( | |
let tcx = relation.tcx(); | ||
|
||
let eagerly_eval = |x: &'tcx ty::Const<'tcx>| { | ||
// FIXME(eddyb) this doesn't account for lifetime inference variables | ||
// being erased by `eval`, *nor* for the polymorphic aspect of `eval`. | ||
// That is, we could always use `eval` and it will just return the | ||
// old value back if it doesn't succeed. | ||
if !x.val.needs_infer() { | ||
return x.eval(tcx, relation.param_env()).val; | ||
} | ||
x.val | ||
}; | ||
|
||
// FIXME(eddyb) doesn't look like everything below checks that `a.ty == b.ty`. | ||
// We could probably always assert it early, as `const` generic parameters | ||
// are not allowed to depend on other generic parameters, i.e. are concrete. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this temporary and won't always remain so, so let's not bake that assumption too deeply into the compiler. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have already, this is nothing compared to everything else. It's only "temporary" in the sense that the first thing we stabilize won't have generic parameters in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of the problem is we'd need to stabilize something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure I understand all of this; I'm just saying we should avoid too deep of an architectural dependence on the fact that const params can't depend on type params so that it doesn't take a year to dig out of. |
||
// (although there could be normalization differences) | ||
|
||
// Currently, the values that can be unified are primitive types, | ||
// and those that derive both `PartialEq` and `Eq`, corresponding | ||
// to structural-match types. | ||
|
@@ -524,6 +533,9 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>( | |
// The caller should handle these cases! | ||
bug!("var types encountered in super_relate_consts: {:?} {:?}", a, b) | ||
} | ||
|
||
(ty::ConstKind::Error, _) | (_, ty::ConstKind::Error) => Ok(ty::ConstKind::Error), | ||
|
||
(ty::ConstKind::Param(a_p), ty::ConstKind::Param(b_p)) if a_p.index == b_p.index => { | ||
return Ok(a); | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.