Rollup merge of #77303 - lcnr:const-evaluatable-TooGeneric, r=oli-obk…

…,varkor

const evaluatable: improve `TooGeneric` handling

Instead of emitting an error in `fulfill`, we now correctly stall on inference variables.

As `const_eval_resolve` returns `ErrorHandled::TooGeneric` when encountering generic parameters on which
we actually do want to error, we check for inference variables and eagerly emit an error if they don't exist, returning `ErrorHandled::Reported` instead.

Also contains a small bugfix for `ConstEquate` where we previously only stalled on type variables. This is probably a leftover from
when we did not yet support stalling on const inference variables.

r? @oli-obk cc @varkor @eddyb

compiler/rustc_trait_selection/src/traits/const_evaluatable.rs

@@ -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)
+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>)) {
+        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>,

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -745,25 +745,9 @@ 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) => {
-                // In this instance, we have a const expression containing an unevaluated
-                // generic parameter. We have no idea whether this expression is valid or
-                // not (e.g. it might result in an error), but we don't want to just assume
-                // that it's okay, because that might result in post-monomorphisation time
-                // errors. The onus is really on the caller to provide values that it can
-                // prove are well-formed.
-                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).
-                err.note("this may fail depending on what value the parameter takes");
-                err
+                bug!("too generic should have been handled in `is_const_evaluatable`");
             }
-
             // Already reported in the query.
             ConstEvalFailure(ErrorHandled::Reported(ErrorReported)) => {
                 // FIXME(eddyb) remove this once `ErrorReported` becomes a proof token.

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -496,6 +496,13 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
                         obligation.cause.span,
                     ) {
                         Ok(()) => ProcessResult::Changed(vec![]),
+                        Err(ErrorHandled::TooGeneric) => {
+                            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)

src/test/ui/const-generics/const_evaluatable_checked/cross_crate_predicate.rs

@@ -5,10 +5,10 @@ extern crate const_evaluatable_lib;
 
 fn user<T>() {
     let _ = const_evaluatable_lib::test1::<T>();
-    //~^ ERROR constant expression depends
-    //~| ERROR constant expression depends
-    //~| ERROR constant expression depends
-    //~| ERROR constant expression depends
+    //~^ ERROR unconstrained generic constant
+    //~| ERROR unconstrained generic constant
+    //~| ERROR unconstrained generic constant
+    //~| ERROR unconstrained generic constant
 }
 
 fn main() {}

src/test/ui/const-generics/const_evaluatable_checked/cross_crate_predicate.stderr

@@ -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
 

src/test/ui/const-generics/const_evaluatable_checked/infer-too-generic.rs

@@ -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]);
+}

src/test/ui/const-generics/issues/issue-76595.rs

@@ -14,5 +14,4 @@ fn test<T, const P: usize>() where Bool<{core::mem::size_of::<T>() > 4}>: True {
 fn main() {
     test::<2>();
     //~^ ERROR wrong number of type
-    //~| ERROR constant expression depends
 }

src/test/ui/const-generics/issues/issue-76595.stderr

@@ -4,17 +4,6 @@ error[E0107]: wrong number of type arguments: expected 1, found 0
 LL |     test::<2>();
    |     ^^^^^^^^^ expected 1 type argument
 
-error: constant expression depends on a generic parameter
-  --> $DIR/issue-76595.rs:15:5
-   |
-LL | fn test<T, const P: usize>() where Bool<{core::mem::size_of::<T>() > 4}>: True {
-   |                                         ------------------------------- required by this bound in `test`
-...
-LL |     test::<2>();
-   |     ^^^^^^^^^
-   |
-   = note: this may fail depending on what value the parameter takes
-
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 
 For more information about this error, try `rustc --explain E0107`.