-
Notifications
You must be signed in to change notification settings - Fork 13.4k
const evaluatable: improve TooGeneric
handling
#77303
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
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 |
---|---|---|
|
@@ -23,6 +23,9 @@ use rustc_session::lint; | |
use rustc_span::def_id::{DefId, LocalDefId}; | ||
use rustc_span::Span; | ||
|
||
use std::cmp; | ||
|
||
/// Check if a given constant can be evaluated. | ||
pub fn is_const_evaluatable<'cx, 'tcx>( | ||
infcx: &InferCtxt<'cx, 'tcx>, | ||
def: ty::WithOptConstParam<DefId>, | ||
|
@@ -32,23 +35,87 @@ pub fn is_const_evaluatable<'cx, 'tcx>( | |
) -> Result<(), ErrorHandled> { | ||
debug!("is_const_evaluatable({:?}, {:?})", def, substs); | ||
if infcx.tcx.features().const_evaluatable_checked { | ||
if let Some(ct) = AbstractConst::new(infcx.tcx, def, substs)? { | ||
for pred in param_env.caller_bounds() { | ||
match pred.skip_binders() { | ||
ty::PredicateAtom::ConstEvaluatable(b_def, b_substs) => { | ||
debug!("is_const_evaluatable: caller_bound={:?}, {:?}", b_def, b_substs); | ||
if b_def == def && b_substs == substs { | ||
debug!("is_const_evaluatable: caller_bound ~~> ok"); | ||
return Ok(()); | ||
} else if AbstractConst::new(infcx.tcx, b_def, b_substs)? | ||
.map_or(false, |b_ct| try_unify(infcx.tcx, ct, b_ct)) | ||
{ | ||
debug!("is_const_evaluatable: abstract_const ~~> ok"); | ||
return Ok(()); | ||
let tcx = infcx.tcx; | ||
match AbstractConst::new(tcx, def, substs)? { | ||
// We are looking at a generic abstract constant. | ||
Some(ct) => { | ||
for pred in param_env.caller_bounds() { | ||
match pred.skip_binders() { | ||
ty::PredicateAtom::ConstEvaluatable(b_def, b_substs) => { | ||
debug!( | ||
"is_const_evaluatable: caller_bound={:?}, {:?}", | ||
b_def, b_substs | ||
); | ||
if b_def == def && b_substs == substs { | ||
debug!("is_const_evaluatable: caller_bound ~~> ok"); | ||
return Ok(()); | ||
} else if AbstractConst::new(tcx, b_def, b_substs)? | ||
.map_or(false, |b_ct| try_unify(tcx, ct, b_ct)) | ||
{ | ||
debug!("is_const_evaluatable: abstract_const ~~> ok"); | ||
return Ok(()); | ||
} | ||
} | ||
_ => {} // don't care | ||
} | ||
_ => {} // don't care | ||
} | ||
|
||
// We were unable to unify the abstract constant with | ||
// a constant found in the caller bounds, there are | ||
// now three possible cases here. | ||
// | ||
// - The substs are concrete enough that we can simply | ||
// try and evaluate the given constant. | ||
// - The abstract const still references an inference | ||
// variable, in this case we return `TooGeneric`. | ||
// - The abstract const references a generic parameter, | ||
// this means that we emit an error here. | ||
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] | ||
enum FailureKind { | ||
MentionsInfer, | ||
MentionsParam, | ||
Concrete, | ||
} | ||
let mut failure_kind = FailureKind::Concrete; | ||
walk_abstract_const(tcx, ct, |node| match node { | ||
Node::Leaf(leaf) => { | ||
let leaf = leaf.subst(tcx, ct.substs); | ||
if leaf.has_infer_types_or_consts() { | ||
failure_kind = FailureKind::MentionsInfer; | ||
} else if leaf.has_param_types_or_consts() { | ||
failure_kind = cmp::min(failure_kind, FailureKind::MentionsParam); | ||
} | ||
} | ||
Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => (), | ||
}); | ||
|
||
match failure_kind { | ||
FailureKind::MentionsInfer => { | ||
return Err(ErrorHandled::TooGeneric); | ||
} | ||
FailureKind::MentionsParam => { | ||
// FIXME(const_evaluatable_checked): Better error message. | ||
infcx | ||
.tcx | ||
.sess | ||
.struct_span_err(span, "unconstrained generic constant") | ||
.span_help( | ||
tcx.def_span(def.did), | ||
"consider adding a `where` bound for this expression", | ||
) | ||
.emit(); | ||
return Err(ErrorHandled::Reported(ErrorReported)); | ||
} | ||
FailureKind::Concrete => { | ||
// Dealt with below by the same code which handles this | ||
// without the feature gate. | ||
} | ||
} | ||
} | ||
None => { | ||
// If we are dealing with a concrete constant, we can | ||
// reuse the old code path and try to evaluate | ||
// the constant. | ||
} | ||
} | ||
} | ||
|
@@ -95,7 +162,36 @@ pub fn is_const_evaluatable<'cx, 'tcx>( | |
} | ||
|
||
debug!(?concrete, "is_const_evaluatable"); | ||
concrete.map(drop) | ||
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)) | ||
} | ||
c => c.map(drop), | ||
} | ||
} | ||
|
||
/// A tree representing an anonymous constant. | ||
|
@@ -421,6 +517,33 @@ pub(super) fn try_unify_abstract_consts<'tcx>( | |
// on `ErrorReported`. | ||
} | ||
|
||
fn walk_abstract_const<'tcx, F>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, mut f: F) | ||
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. is there any particular reason we need to walk this in tree order and not just visit all nodes? 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. So for subtrees that would be incorrect, as they can contain nodes from other subtrees. I think if we always use a method like this for a complete tree that would be fine. Kinda afraid that we accidentially incluse unused nodes though 🤔 About unused nodes, do we want to emit an error if we encounter them during 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. #![feature(const_generics, const_evaluatable_checked)]
fn test<const N: usize>() -> usize {
N; 2
} results in
which is currently allowed. Do we want to forbid unused nodes in abstract const building? edit: I think we do, will open a PR for that (once this is merged as I want to reuse |
||
where | ||
F: FnMut(Node<'tcx>), | ||
{ | ||
recurse(tcx, ct, &mut f); | ||
fn recurse<'tcx>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, f: &mut dyn FnMut(Node<'tcx>)) { | ||
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. any kind of recursion should probably use 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. Will we ever get really deep trees here? 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. It may occur in generated code somewhere, and if the rest of the compiler code is already recursion depth resistant we can hit this here. Once this is merged and in nightly I'll try to build a test that hits this. |
||
let root = ct.root(); | ||
f(root); | ||
match root { | ||
Node::Leaf(_) => (), | ||
Node::Binop(_, l, r) => { | ||
recurse(tcx, ct.subtree(l), f); | ||
recurse(tcx, ct.subtree(r), f); | ||
} | ||
Node::UnaryOp(_, v) => { | ||
recurse(tcx, ct.subtree(v), f); | ||
} | ||
Node::FunctionCall(func, args) => { | ||
recurse(tcx, ct.subtree(func), f); | ||
for &arg in args { | ||
recurse(tcx, ct.subtree(arg), f); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
/// Tries to unify two abstract constants using structural equality. | ||
pub(super) fn try_unify<'tcx>( | ||
tcx: TyCtxt<'tcx>, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -496,6 +496,13 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> { | |
obligation.cause.span, | ||
) { | ||
Ok(()) => ProcessResult::Changed(vec![]), | ||
Err(ErrorHandled::TooGeneric) => { | ||
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 don't know anything about this part of 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. |
||
pending_obligation.stalled_on = substs | ||
.iter() | ||
.filter_map(|ty| TyOrConstInferVar::maybe_from_generic_arg(ty)) | ||
.collect(); | ||
ProcessResult::Unchanged | ||
} | ||
Err(e) => ProcessResult::Error(CodeSelectionError(ConstEvalFailure(e))), | ||
} | ||
} | ||
|
@@ -537,8 +544,10 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> { | |
Err(ErrorHandled::TooGeneric) => { | ||
stalled_on.append( | ||
&mut substs | ||
.types() | ||
.filter_map(|ty| TyOrConstInferVar::maybe_from_ty(ty)) | ||
.iter() | ||
.filter_map(|arg| { | ||
TyOrConstInferVar::maybe_from_generic_arg(arg) | ||
}) | ||
.collect(), | ||
); | ||
Err(ErrorHandled::TooGeneric) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,54 +1,50 @@ | ||
error: constant expression depends on a generic parameter | ||
error: unconstrained generic constant | ||
--> $DIR/cross_crate_predicate.rs:7:13 | ||
| | ||
LL | let _ = const_evaluatable_lib::test1::<T>(); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
::: $DIR/auxiliary/const_evaluatable_lib.rs:6:10 | ||
| | ||
LL | [u8; std::mem::size_of::<T>() - 1]: Sized, | ||
| ---------------------------- required by this bound in `test1` | ||
help: consider adding a `where` bound for this expression | ||
--> $DIR/auxiliary/const_evaluatable_lib.rs:6:10 | ||
| | ||
= note: this may fail depending on what value the parameter takes | ||
LL | [u8; std::mem::size_of::<T>() - 1]: Sized, | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: constant expression depends on a generic parameter | ||
error: unconstrained generic constant | ||
--> $DIR/cross_crate_predicate.rs:7:13 | ||
| | ||
LL | let _ = const_evaluatable_lib::test1::<T>(); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
::: $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: consider adding a `where` bound for this expression | ||
--> $DIR/auxiliary/const_evaluatable_lib.rs:4:27 | ||
| | ||
= note: this may fail depending on what value the parameter takes | ||
LL | pub fn test1<T>() -> [u8; std::mem::size_of::<T>() - 1] | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: constant expression depends on a generic parameter | ||
error: unconstrained generic constant | ||
--> $DIR/cross_crate_predicate.rs:7:13 | ||
| | ||
LL | let _ = const_evaluatable_lib::test1::<T>(); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
::: $DIR/auxiliary/const_evaluatable_lib.rs:6:10 | ||
| | ||
LL | [u8; std::mem::size_of::<T>() - 1]: Sized, | ||
| ---------------------------- required by this bound in `test1` | ||
help: consider adding a `where` bound for this expression | ||
--> $DIR/auxiliary/const_evaluatable_lib.rs:6:10 | ||
| | ||
= note: this may fail depending on what value the parameter takes | ||
LL | [u8; std::mem::size_of::<T>() - 1]: Sized, | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: constant expression depends on a generic parameter | ||
error: unconstrained generic constant | ||
--> $DIR/cross_crate_predicate.rs:7:13 | ||
| | ||
LL | let _ = const_evaluatable_lib::test1::<T>(); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
::: $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: consider adding a `where` bound for this expression | ||
--> $DIR/auxiliary/const_evaluatable_lib.rs:4:27 | ||
| | ||
= note: this may fail depending on what value the parameter takes | ||
LL | pub fn test1<T>() -> [u8; std::mem::size_of::<T>() - 1] | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to 4 previous errors | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// run-pass | ||
#![feature(const_generics, const_evaluatable_checked)] | ||
#![allow(incomplete_features)] | ||
|
||
use std::{mem, ptr}; | ||
|
||
fn split_first<T, const N: usize>(arr: [T; N]) -> (T, [T; N - 1]) | ||
where | ||
[T; N - 1]: Sized, | ||
{ | ||
let arr = mem::ManuallyDrop::new(arr); | ||
unsafe { | ||
let head = ptr::read(&arr[0]); | ||
let tail = ptr::read(&arr[1..] as *const [T] as *const [T; N - 1]); | ||
(head, tail) | ||
} | ||
} | ||
|
||
fn main() { | ||
let arr = [0, 1, 2, 3, 4]; | ||
let (head, tail) = split_first(arr); | ||
assert_eq!(head, 0); | ||
assert_eq!(tail, [1, 2, 3, 4]); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.