Skip to content

Commit

Permalink
Rollup merge of #82707 - BoxyUwU:errooaaar, r=oli-obk
Browse files Browse the repository at this point in the history
const_evaluatable_checked: Stop eagerly erroring in `is_const_evaluatable`

Fixes #82279

We don't want to be emitting errors inside of is_const_evaluatable because we may call this during selection where it should be able to fail silently

There were two errors being emitted in `is_const_evaluatable`. The one causing the compile error in #82279 was inside the match arm for `FailureKind::MentionsParam` but I moved the other error being emitted too since it made things cleaner imo

The `NotConstEvaluatable` enum \*should\* have a fourth variant for when we fail to evaluate a concrete const, e.g. `0 - 1` but that cant happen until #81339

cc `@oli-obk` `@lcnr`
r? `@nikomatsakis`
  • Loading branch information
Dylan-DPC committed Mar 21, 2021
2 parents 61edfd5 + 8e353bb commit 3a113f1
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 109 deletions.
17 changes: 17 additions & 0 deletions compiler/rustc_middle/src/mir/abstract_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,20 @@ pub enum Node<'tcx> {
UnaryOp(mir::UnOp, NodeId),
FunctionCall(NodeId, &'tcx [NodeId]),
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)]
pub enum NotConstEvaluatable {
Error(rustc_errors::ErrorReported),
MentionsInfer,
MentionsParam,
}

impl From<rustc_errors::ErrorReported> for NotConstEvaluatable {
fn from(e: rustc_errors::ErrorReported) -> NotConstEvaluatable {
NotConstEvaluatable::Error(e)
}
}

TrivialTypeFoldableAndLiftImpls! {
NotConstEvaluatable,
}
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub mod specialization_graph;
mod structural_impls;

use crate::infer::canonical::Canonical;
use crate::mir::interpret::ErrorHandled;
use crate::mir::abstract_const::NotConstEvaluatable;
use crate::ty::subst::SubstsRef;
use crate::ty::{self, AdtKind, Ty, TyCtxt};

Expand Down Expand Up @@ -398,7 +398,7 @@ pub enum SelectionError<'tcx> {
ty::error::TypeError<'tcx>,
),
TraitNotObjectSafe(DefId),
ConstEvalFailure(ErrorHandled),
NotConstEvaluatable(NotConstEvaluatable),
Overflow,
}

Expand Down
63 changes: 13 additions & 50 deletions compiler/rustc_trait_selection/src/traits/const_evaluatable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_hir::def::DefKind;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;
use rustc_infer::infer::InferCtxt;
use rustc_middle::mir::abstract_const::{Node, NodeId};
use rustc_middle::mir::abstract_const::{Node, NodeId, NotConstEvaluatable};
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::mir::{self, Rvalue, StatementKind, TerminatorKind};
use rustc_middle::ty::subst::{Subst, SubstsRef};
Expand All @@ -32,7 +32,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
substs: SubstsRef<'tcx>,
param_env: ty::ParamEnv<'tcx>,
span: Span,
) -> Result<(), ErrorHandled> {
) -> Result<(), NotConstEvaluatable> {
debug!("is_const_evaluatable({:?}, {:?})", def, substs);
if infcx.tcx.features().const_evaluatable_checked {
let tcx = infcx.tcx;
Expand Down Expand Up @@ -103,29 +103,10 @@ pub fn is_const_evaluatable<'cx, 'tcx>(

match failure_kind {
FailureKind::MentionsInfer => {
return Err(ErrorHandled::TooGeneric);
return Err(NotConstEvaluatable::MentionsInfer);
}
FailureKind::MentionsParam => {
// FIXME(const_evaluatable_checked): Better error message.
let mut err =
infcx.tcx.sess.struct_span_err(span, "unconstrained generic constant");
let const_span = tcx.def_span(def.did);
// FIXME(const_evaluatable_checked): Update this suggestion once
// explicit const evaluatable bounds are implemented.
if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(const_span)
{
err.span_help(
tcx.def_span(def.did),
&format!("try adding a `where` bound using this expression: `where [u8; {}]: Sized`", snippet),
);
} else {
err.span_help(
const_span,
"consider adding a `where` bound for this expression",
);
}
err.emit();
return Err(ErrorHandled::Reported(ErrorReported));
return Err(NotConstEvaluatable::MentionsParam);
}
FailureKind::Concrete => {
// Dealt with below by the same code which handles this
Expand Down Expand Up @@ -180,34 +161,16 @@ pub fn is_const_evaluatable<'cx, 'tcx>(

debug!(?concrete, "is_const_evaluatable");
match concrete {
Err(ErrorHandled::TooGeneric) if !substs.has_infer_types_or_consts() => {
// FIXME(const_evaluatable_checked): We really should move
// emitting this error message to fulfill instead. For
// now this is easier.
//
// This is not a problem without `const_evaluatable_checked` as
// all `ConstEvaluatable` predicates have to be fulfilled for compilation
// to succeed.
//
// @lcnr: We already emit an error for things like
// `fn test<const N: usize>() -> [0 - N]` eagerly here,
// so until we fix this I don't really care.

let mut err = infcx
.tcx
.sess
.struct_span_err(span, "constant expression depends on a generic parameter");
// FIXME(const_generics): we should suggest to the user how they can resolve this
// issue. However, this is currently not actually possible
// (see https://github.com/rust-lang/rust/issues/66962#issuecomment-575907083).
//
// Note that with `feature(const_evaluatable_checked)` this case should not
// be reachable.
err.note("this may fail depending on what value the parameter takes");
err.emit();
Err(ErrorHandled::Reported(ErrorReported))
Err(ErrorHandled::TooGeneric) => Err(match substs.has_infer_types_or_consts() {
true => NotConstEvaluatable::MentionsInfer,
false => NotConstEvaluatable::MentionsParam,
}),
Err(ErrorHandled::Linted) => {
infcx.tcx.sess.delay_span_bug(span, "constant in type had error reported as lint");
Err(NotConstEvaluatable::Error(ErrorReported))
}
c => c.map(drop),
Err(ErrorHandled::Reported(e)) => Err(NotConstEvaluatable::Error(e)),
Ok(_) => Ok(()),
}
}

Expand Down
67 changes: 51 additions & 16 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ pub mod on_unimplemented;
pub mod suggestions;

use super::{
ConstEvalFailure, EvaluationResult, FulfillmentError, FulfillmentErrorCode,
MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode,
OnUnimplementedDirective, OnUnimplementedNote, OutputTypeParameterMismatch, Overflow,
PredicateObligation, SelectionContext, SelectionError, TraitNotObjectSafe,
EvaluationResult, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes,
Obligation, ObligationCause, ObligationCauseCode, OnUnimplementedDirective,
OnUnimplementedNote, OutputTypeParameterMismatch, Overflow, PredicateObligation,
SelectionContext, SelectionError, TraitNotObjectSafe,
};

use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
Expand All @@ -17,7 +17,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::intravisit::Visitor;
use rustc_hir::Node;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::mir::abstract_const::NotConstEvaluatable;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::fold::TypeFolder;
use rustc_middle::ty::{
Expand Down Expand Up @@ -738,24 +738,59 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let violations = self.tcx.object_safety_violations(did);
report_object_safety_error(self.tcx, span, did, violations)
}
ConstEvalFailure(ErrorHandled::TooGeneric) => {
bug!("too generic should have been handled in `is_const_evaluatable`");

SelectionError::NotConstEvaluatable(NotConstEvaluatable::MentionsInfer) => {
bug!(
"MentionsInfer should have been handled in `traits/fulfill.rs` or `traits/select/mod.rs`"
)
}
SelectionError::NotConstEvaluatable(NotConstEvaluatable::MentionsParam) => {
if !self.tcx.features().const_evaluatable_checked {
let mut err = self.tcx.sess.struct_span_err(
span,
"constant expression depends on a generic parameter",
);
// FIXME(const_generics): we should suggest to the user how they can resolve this
// issue. However, this is currently not actually possible
// (see https://github.com/rust-lang/rust/issues/66962#issuecomment-575907083).
//
// Note that with `feature(const_evaluatable_checked)` this case should not
// be reachable.
err.note("this may fail depending on what value the parameter takes");
err.emit();
return;
}

match obligation.predicate.kind().skip_binder() {
ty::PredicateKind::ConstEvaluatable(def, _) => {
let mut err =
self.tcx.sess.struct_span_err(span, "unconstrained generic constant");
let const_span = self.tcx.def_span(def.did);
match self.tcx.sess.source_map().span_to_snippet(const_span) {
Ok(snippet) => err.help(&format!(
"try adding a `where` bound using this expression: `where [(); {}]:`",
snippet
)),
_ => err.help("consider adding a `where` bound using this expression"),
};
err
}
_ => {
span_bug!(
span,
"unexpected non-ConstEvaluatable predicate, this should not be reachable"
)
}
}
}

// Already reported in the query.
ConstEvalFailure(ErrorHandled::Reported(ErrorReported)) => {
SelectionError::NotConstEvaluatable(NotConstEvaluatable::Error(ErrorReported)) => {
// FIXME(eddyb) remove this once `ErrorReported` becomes a proof token.
self.tcx.sess.delay_span_bug(span, "`ErrorReported` without an error");
return;
}

// Already reported in the query, but only as a lint.
// This shouldn't actually happen for constants used in types, modulo
// bugs. The `delay_span_bug` here ensures it won't be ignored.
ConstEvalFailure(ErrorHandled::Linted) => {
self.tcx.sess.delay_span_bug(span, "constant in type had error reported as lint");
return;
}

Overflow => {
bug!("overflow should be handled before the `report_selection_error` path");
}
Expand Down
24 changes: 15 additions & 9 deletions compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use rustc_data_structures::obligation_forest::ProcessResult;
use rustc_data_structures::obligation_forest::{Error, ForestObligation, Outcome};
use rustc_data_structures::obligation_forest::{ObligationForest, ObligationProcessor};
use rustc_errors::ErrorReported;
use rustc_infer::traits::{TraitEngine, TraitEngineExt as _, TraitObligation};
use rustc_infer::traits::{SelectionError, TraitEngine, TraitEngineExt as _, TraitObligation};
use rustc_middle::mir::abstract_const::NotConstEvaluatable;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::subst::SubstsRef;
Expand All @@ -18,7 +19,7 @@ use super::wf;
use super::CodeAmbiguity;
use super::CodeProjectionError;
use super::CodeSelectionError;
use super::{ConstEvalFailure, Unimplemented};
use super::Unimplemented;
use super::{FulfillmentError, FulfillmentErrorCode};
use super::{ObligationCause, PredicateObligation};

Expand Down Expand Up @@ -498,14 +499,19 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
obligation.cause.span,
) {
Ok(()) => ProcessResult::Changed(vec![]),
Err(ErrorHandled::TooGeneric) => {
Err(NotConstEvaluatable::MentionsInfer) => {
pending_obligation.stalled_on.clear();
pending_obligation.stalled_on.extend(
substs.iter().filter_map(TyOrConstInferVar::maybe_from_generic_arg),
);
ProcessResult::Unchanged
}
Err(e) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(e))),
Err(
e @ NotConstEvaluatable::MentionsParam
| e @ NotConstEvaluatable::Error(_),
) => ProcessResult::Error(CodeSelectionError(
SelectionError::NotConstEvaluatable(e),
)),
}
}

Expand Down Expand Up @@ -576,11 +582,11 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
}
}
(Err(ErrorHandled::Reported(ErrorReported)), _)
| (_, Err(ErrorHandled::Reported(ErrorReported))) => {
ProcessResult::Error(CodeSelectionError(ConstEvalFailure(
ErrorHandled::Reported(ErrorReported),
)))
}
| (_, Err(ErrorHandled::Reported(ErrorReported))) => ProcessResult::Error(
CodeSelectionError(SelectionError::NotConstEvaluatable(
NotConstEvaluatable::Error(ErrorReported),
)),
),
(Err(ErrorHandled::Linted), _) | (_, Err(ErrorHandled::Linted)) => {
span_bug!(
obligation.cause.span(self.selcx.tcx()),
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use rustc_hir::def_id::DefId;
use rustc_hir::Constness;
use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::dep_graph::{DepKind, DepNodeIndex};
use rustc_middle::mir::abstract_const::NotConstEvaluatable;
use rustc_middle::mir::interpret::ErrorHandled;
use rustc_middle::ty::fast_reject;
use rustc_middle::ty::print::with_no_trimmed_paths;
Expand Down Expand Up @@ -547,7 +548,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation.cause.span,
) {
Ok(()) => Ok(EvaluatedToOk),
Err(ErrorHandled::TooGeneric) => Ok(EvaluatedToAmbig),
Err(NotConstEvaluatable::MentionsInfer) => Ok(EvaluatedToAmbig),
Err(NotConstEvaluatable::MentionsParam) => Ok(EvaluatedToErr),
Err(_) => Ok(EvaluatedToErr),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,52 @@ error: unconstrained generic constant
|
LL | let _ = const_evaluatable_lib::test1::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try adding a `where` bound using this expression: `where [u8; std::mem::size_of::<T>() - 1]: Sized`
--> $DIR/auxiliary/const_evaluatable_lib.rs:6:10
|
::: $DIR/auxiliary/const_evaluatable_lib.rs:6:10
|
LL | [u8; std::mem::size_of::<T>() - 1]: Sized,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ---------------------------- required by this bound in `test1`
|
= help: try adding a `where` bound using this expression: `where [(); std::mem::size_of::<T>() - 1]:`

error: unconstrained generic constant
--> $DIR/cross_crate_predicate.rs:7:13
|
LL | let _ = const_evaluatable_lib::test1::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try adding a `where` bound using this expression: `where [u8; std::mem::size_of::<T>() - 1]: Sized`
--> $DIR/auxiliary/const_evaluatable_lib.rs:4:27
|
::: $DIR/auxiliary/const_evaluatable_lib.rs:4:27
|
LL | pub fn test1<T>() -> [u8; std::mem::size_of::<T>() - 1]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ---------------------------- required by this bound in `test1`
|
= help: try adding a `where` bound using this expression: `where [(); std::mem::size_of::<T>() - 1]:`

error: unconstrained generic constant
--> $DIR/cross_crate_predicate.rs:7:13
|
LL | let _ = const_evaluatable_lib::test1::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try adding a `where` bound using this expression: `where [u8; std::mem::size_of::<T>() - 1]: Sized`
--> $DIR/auxiliary/const_evaluatable_lib.rs:6:10
|
::: $DIR/auxiliary/const_evaluatable_lib.rs:6:10
|
LL | [u8; std::mem::size_of::<T>() - 1]: Sized,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ---------------------------- required by this bound in `test1`
|
= help: try adding a `where` bound using this expression: `where [(); std::mem::size_of::<T>() - 1]:`

error: unconstrained generic constant
--> $DIR/cross_crate_predicate.rs:7:13
|
LL | let _ = const_evaluatable_lib::test1::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try adding a `where` bound using this expression: `where [u8; std::mem::size_of::<T>() - 1]: Sized`
--> $DIR/auxiliary/const_evaluatable_lib.rs:4:27
|
::: $DIR/auxiliary/const_evaluatable_lib.rs:4:27
|
LL | pub fn test1<T>() -> [u8; std::mem::size_of::<T>() - 1]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ---------------------------- required by this bound in `test1`
|
= help: try adding a `where` bound using this expression: `where [(); std::mem::size_of::<T>() - 1]:`

error: aborting due to 4 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ error: unconstrained generic constant
LL | [0; size_of::<Foo<T>>()]
| ^^^^^^^^^^^^^^^^^^^
|
help: try adding a `where` bound using this expression: `where [u8; size_of::<Foo<T>>()]: Sized`
--> $DIR/different-fn.rs:10:9
|
LL | [0; size_of::<Foo<T>>()]
| ^^^^^^^^^^^^^^^^^^^
= help: try adding a `where` bound using this expression: `where [(); size_of::<Foo<T>>()]:`

error: aborting due to previous error

Loading

0 comments on commit 3a113f1

Please sign in to comment.